Fix tests for LSPS4 and Logger generic additions (#19)

LiquidityManager gained an 8th generic parameter (Logger) and
KVStore targets now require KVStoreSync. Several test files and
doctests were not updated after these changes landed.

background-processor: The NO_LIQUIDITY_MANAGER const uses a dyn
trait object for KVStore, but trait objects can only name one
trait. Add a KVStoreFull supertrait combining KVStore and
KVStoreSync so the dyn bound remains expressible. Add the Logger
generic and KVStoreSync impl to the process_events_async
doctest's stub Store type.

lightning-liquidity tests: Add missing lsps4_service_config and
lsps4_client_config fields (None) to config initializers in
lsps0, lsps2, and lsps5 integration tests. Pass logger to
LiquidityManagerSync::new calls and add TestLogger to type
annotations.

channel.rs tests: Remove duplicate import block left over from
a bad merge in 736d3a8. Fix field accesses that moved from
ChannelContext to FundingScope (channel_value_satoshis,
holder_selected_channel_reserve_satoshis). Update
TestFeeEstimator construction to use the new constructor API.

Author: amackillop

lightning-background-processor/src/lib.rs

@@ -416,6 +416,11 @@ pub const NO_ONION_MESSENGER: Option<
 	>,
 > = None;
 
+/// Supertrait combining [`KVStore`] and [`KVStoreSync`] so that a single `dyn` trait object can
+/// satisfy the bounds required by [`ALiquidityManager`].
+pub trait KVStoreFull: KVStore + KVStoreSync {}
+impl<T: KVStore + KVStoreSync + ?Sized> KVStoreFull for T {}
+
 /// When initializing a background processor without a liquidity manager, this can be used to avoid
 /// specifying a concrete `LiquidityManager` type.
 #[cfg(not(c_bindings))]
@@ -430,8 +435,8 @@ pub const NO_LIQUIDITY_MANAGER: Option<
 				CM = &DynChannelManager,
 				Filter = dyn chain::Filter + Send + Sync,
 				C = &(dyn chain::Filter + Send + Sync),
-				KVStore = dyn lightning::util::persist::KVStore + Send + Sync,
-				K = &(dyn lightning::util::persist::KVStore + Send + Sync),
+				KVStore = dyn KVStoreFull + Send + Sync,
+				K = &(dyn KVStoreFull + Send + Sync),
 				TimeProvider = dyn lightning_liquidity::utils::time::TimeProvider + Send + Sync,
 				TP = &(dyn lightning_liquidity::utils::time::TimeProvider + Send + Sync),
 				BroadcasterInterface = dyn lightning::chain::chaininterface::BroadcasterInterface
@@ -762,6 +767,12 @@ use futures_util::{dummy_waker, Joiner, OptionalSelector, Selector, SelectorOutp
 /// #     fn remove(&self, primary_namespace: &str, secondary_namespace: &str, key: &str, lazy: bool) -> Pin<Box<dyn Future<Output = Result<(), io::Error>> + 'static + Send>> { todo!() }
 /// #     fn list(&self, primary_namespace: &str, secondary_namespace: &str) -> Pin<Box<dyn Future<Output = Result<Vec<String>, io::Error>> + 'static + Send>> { todo!() }
 /// # }
+/// # impl lightning::util::persist::KVStoreSync for Store {
+/// #     fn read(&self, primary_namespace: &str, secondary_namespace: &str, key: &str) -> io::Result<Vec<u8>> { Ok(Vec::new()) }
+/// #     fn write(&self, primary_namespace: &str, secondary_namespace: &str, key: &str, buf: Vec<u8>) -> io::Result<()> { Ok(()) }
+/// #     fn remove(&self, primary_namespace: &str, secondary_namespace: &str, key: &str, lazy: bool) -> io::Result<()> { Ok(()) }
+/// #     fn list(&self, primary_namespace: &str, secondary_namespace: &str) -> io::Result<Vec<String>> { Ok(Vec::new()) }
+/// # }
 /// # use core::time::Duration;
 /// # struct DefaultTimeProvider;
 /// #
@@ -786,7 +797,7 @@ use futures_util::{dummy_waker, Joiner, OptionalSelector, Selector, SelectorOutp
 /// # type P2PGossipSync<UL> = lightning::routing::gossip::P2PGossipSync<Arc<NetworkGraph>, Arc<UL>, Arc<Logger>>;
 /// # type ChannelManager<B, F, FE> = lightning::ln::channelmanager::SimpleArcChannelManager<ChainMonitor<B, F, FE>, B, FE, Logger>;
 /// # type OnionMessenger<B, F, FE> = lightning::onion_message::messenger::OnionMessenger<Arc<lightning::sign::KeysManager>, Arc<lightning::sign::KeysManager>, Arc<Logger>, Arc<ChannelManager<B, F, FE>>, Arc<lightning::onion_message::messenger::DefaultMessageRouter<Arc<NetworkGraph>, Arc<Logger>, Arc<lightning::sign::KeysManager>>>, Arc<ChannelManager<B, F, FE>>, lightning::ln::peer_handler::IgnoringMessageHandler, lightning::ln::peer_handler::IgnoringMessageHandler, lightning::ln::peer_handler::IgnoringMessageHandler>;
-/// # type LiquidityManager<B, F, FE> = lightning_liquidity::LiquidityManager<Arc<lightning::sign::KeysManager>, Arc<lightning::sign::KeysManager>, Arc<ChannelManager<B, F, FE>>, Arc<F>, Arc<Store>, Arc<DefaultTimeProvider>, Arc<B>>;
+/// # type LiquidityManager<B, F, FE> = lightning_liquidity::LiquidityManager<Arc<lightning::sign::KeysManager>, Arc<lightning::sign::KeysManager>, Arc<ChannelManager<B, F, FE>>, Arc<F>, Arc<Store>, Arc<DefaultTimeProvider>, Arc<B>, Arc<Logger>>;
 /// # type Scorer = RwLock<lightning::routing::scoring::ProbabilisticScorer<Arc<NetworkGraph>, Arc<Logger>>>;
 /// # type PeerManager<B, F, FE, UL> = lightning::ln::peer_handler::SimpleArcPeerManager<SocketDescriptor, ChainMonitor<B, F, FE>, B, FE, Arc<UL>, Logger, F, StoreSync>;
 /// # type OutputSweeper<B, D, FE, F, O> = lightning::util::sweep::OutputSweeper<Arc<B>, Arc<D>, Arc<FE>, Arc<F>, Arc<Store>, Arc<Logger>, Arc<O>>;
@@ -1967,6 +1978,7 @@ mod tests {
 		Arc<Persister>,
 		DefaultTimeProvider,
 		Arc<test_utils::TestBroadcaster>,
+		Arc<test_utils::TestLogger>,
 	>;
 
 	struct Node {
@@ -2426,6 +2438,7 @@ mod tests {
 					Arc::clone(&tx_broadcaster),
 					None,
 					None,
+					Arc::clone(&logger),
 				)
 				.unwrap(),
 			);
@@ -2798,10 +2811,10 @@ mod tests {
 		let kv_store = KVStoreSyncWrapper(kv_store_sync);
 
 		// Yes, you can unsafe { turn off the borrow checker }
-		let lm_async: &'static LiquidityManager<_, _, _, _, _, _, _> = unsafe {
+		let lm_async: &'static LiquidityManager<_, _, _, _, _, _, _, _> = unsafe {
 			&*(nodes[0].liquidity_manager.get_lm_async()
-				as *const LiquidityManager<_, _, _, _, _, _, _>)
-				as &'static LiquidityManager<_, _, _, _, _, _, _>
+				as *const LiquidityManager<_, _, _, _, _, _, _, _>)
+				as &'static LiquidityManager<_, _, _, _, _, _, _, _>
 		};
 		let sweeper_async: &'static OutputSweeper<_, _, _, _, _, _, _> = unsafe {
 			&*(nodes[0].sweeper.sweeper_async() as *const OutputSweeper<_, _, _, _, _, _, _>)
@@ -3317,10 +3330,10 @@ mod tests {
 		let kv_store = KVStoreSyncWrapper(kv_store_sync);
 
 		// Yes, you can unsafe { turn off the borrow checker }
-		let lm_async: &'static LiquidityManager<_, _, _, _, _, _, _> = unsafe {
+		let lm_async: &'static LiquidityManager<_, _, _, _, _, _, _, _> = unsafe {
 			&*(nodes[0].liquidity_manager.get_lm_async()
-				as *const LiquidityManager<_, _, _, _, _, _, _>)
-				as &'static LiquidityManager<_, _, _, _, _, _, _>
+				as *const LiquidityManager<_, _, _, _, _, _, _, _>)
+				as &'static LiquidityManager<_, _, _, _, _, _, _, _>
 		};
 		let sweeper_async: &'static OutputSweeper<_, _, _, _, _, _, _> = unsafe {
 			&*(nodes[0].sweeper.sweeper_async() as *const OutputSweeper<_, _, _, _, _, _, _>)
@@ -3544,10 +3557,10 @@ mod tests {
 		let (exit_sender, exit_receiver) = tokio::sync::watch::channel(());
 
 		// Yes, you can unsafe { turn off the borrow checker }
-		let lm_async: &'static LiquidityManager<_, _, _, _, _, _, _> = unsafe {
+		let lm_async: &'static LiquidityManager<_, _, _, _, _, _, _, _> = unsafe {
 			&*(nodes[0].liquidity_manager.get_lm_async()
-				as *const LiquidityManager<_, _, _, _, _, _, _>)
-				as &'static LiquidityManager<_, _, _, _, _, _, _>
+				as *const LiquidityManager<_, _, _, _, _, _, _, _>)
+				as &'static LiquidityManager<_, _, _, _, _, _, _, _>
 		};
 		let sweeper_async: &'static OutputSweeper<_, _, _, _, _, _, _> = unsafe {
 			&*(nodes[0].sweeper.sweeper_async() as *const OutputSweeper<_, _, _, _, _, _, _>)

lightning-liquidity/tests/common/mod.rs

@@ -6,7 +6,7 @@ use lightning_liquidity::{LiquidityClientConfig, LiquidityManagerSync, Liquidity
 use lightning::chain::{BestBlock, Filter};
 use lightning::ln::channelmanager::ChainParameters;
 use lightning::ln::functional_test_utils::{Node, TestChannelManager};
-use lightning::util::test_utils::{TestBroadcaster, TestKeysInterface, TestStore};
+use lightning::util::test_utils::{TestBroadcaster, TestKeysInterface, TestLogger, TestStore};
 
 use bitcoin::Network;
 
@@ -47,6 +47,7 @@ fn build_service_and_client_nodes<'a, 'b, 'c>(
 		Some(service_config),
 		None,
 		Arc::clone(&time_provider),
+		service_inner.logger,
 	)
 	.unwrap();
 
@@ -61,6 +62,7 @@ fn build_service_and_client_nodes<'a, 'b, 'c>(
 		None,
 		Some(client_config),
 		time_provider,
+		client_inner.logger,
 	)
 	.unwrap();
 
@@ -141,6 +143,7 @@ pub(crate) struct LiquidityNode<'a, 'b, 'c> {
 		Arc<TestStore>,
 		Arc<dyn TimeProvider + Send + Sync>,
 		&'c TestBroadcaster,
+		&'c TestLogger,
 	>,
 }
 
@@ -155,6 +158,7 @@ impl<'a, 'b, 'c> LiquidityNode<'a, 'b, 'c> {
 			Arc<TestStore>,
 			Arc<dyn TimeProvider + Send + Sync>,
 			&'c TestBroadcaster,
+			&'c TestLogger,
 		>,
 	) -> Self {
 		Self { inner: node, liquidity_manager }

lightning-liquidity/tests/lsps0_integration_tests.rs

@@ -40,6 +40,7 @@ fn list_protocols_integration_test() {
 		#[cfg(lsps1_service)]
 		lsps1_service_config: Some(lsps1_service_config),
 		lsps2_service_config: Some(lsps2_service_config),
+		lsps4_service_config: None,
 		lsps5_service_config: Some(lsps5_service_config),
 		advertise_service: true,
 	};
@@ -54,6 +55,7 @@ fn list_protocols_integration_test() {
 		#[cfg(not(lsps1_service))]
 		lsps1_client_config: None,
 		lsps2_client_config: Some(lsps2_client_config),
+		lsps4_client_config: None,
 		lsps5_client_config: Some(lsps5_client_config),
 	};
 

lightning-liquidity/tests/lsps2_integration_tests.rs

@@ -73,6 +73,7 @@ fn build_lsps2_configs() -> ([u8; 32], LiquidityServiceConfig, LiquidityClientCo
 		#[cfg(lsps1_service)]
 		lsps1_service_config: None,
 		lsps2_service_config: Some(lsps2_service_config),
+		lsps4_service_config: None,
 		lsps5_service_config: None,
 		advertise_service: true,
 	};
@@ -81,6 +82,7 @@ fn build_lsps2_configs() -> ([u8; 32], LiquidityServiceConfig, LiquidityClientCo
 	let client_config = LiquidityClientConfig {
 		lsps1_client_config: None,
 		lsps2_client_config: Some(lsps2_client_config),
+		lsps4_client_config: None,
 		lsps5_client_config: None,
 	};
 
@@ -958,6 +960,7 @@ fn lsps2_service_handler_persistence_across_restarts() {
 		#[cfg(lsps1_service)]
 		lsps1_service_config: None,
 		lsps2_service_config: Some(LSPS2ServiceConfig { promise_secret }),
+		lsps4_service_config: None,
 		lsps5_service_config: None,
 		advertise_service: true,
 	};
@@ -1102,6 +1105,7 @@ fn lsps2_service_handler_persistence_across_restarts() {
 			Some(service_config),
 			None,
 			time_provider,
+			nodes_restart[0].logger,
 		)
 		.unwrap();
 

lightning-liquidity/tests/lsps5_integration_tests.rs

@@ -59,6 +59,7 @@ pub(crate) fn lsps5_test_setup_with_kv_stores<'a, 'b, 'c>(
 		#[cfg(lsps1_service)]
 		lsps1_service_config: None,
 		lsps2_service_config: None,
+		lsps4_service_config: None,
 		lsps5_service_config: Some(lsps5_service_config),
 		advertise_service: true,
 	};
@@ -68,6 +69,7 @@ pub(crate) fn lsps5_test_setup_with_kv_stores<'a, 'b, 'c>(
 	let client_config = LiquidityClientConfig {
 		lsps1_client_config: None,
 		lsps2_client_config: None,
+		lsps4_client_config: None,
 		lsps5_client_config: Some(lsps5_client_config),
 	};
 
@@ -243,6 +245,7 @@ pub(crate) fn lsps5_lsps2_test_setup<'a, 'b, 'c>(
 		#[cfg(lsps1_service)]
 		lsps1_service_config: None,
 		lsps2_service_config: Some(lsps2_service_config),
+		lsps4_service_config: None,
 		lsps5_service_config: Some(lsps5_service_config),
 		advertise_service: true,
 	};
@@ -252,6 +255,7 @@ pub(crate) fn lsps5_lsps2_test_setup<'a, 'b, 'c>(
 	let client_config = LiquidityClientConfig {
 		lsps1_client_config: None,
 		lsps2_client_config: Some(lsps2_client_config),
+		lsps4_client_config: None,
 		lsps5_client_config: Some(lsps5_client_config),
 	};
 
@@ -1519,6 +1523,7 @@ fn lsps5_service_handler_persistence_across_restarts() {
 		#[cfg(lsps1_service)]
 		lsps1_service_config: None,
 		lsps2_service_config: None,
+		lsps4_service_config: None,
 		lsps5_service_config: Some(LSPS5ServiceConfig::default()),
 		advertise_service: true,
 	};
@@ -1619,6 +1624,7 @@ fn lsps5_service_handler_persistence_across_restarts() {
 			Some(service_config),
 			None,
 			Arc::clone(&time_provider),
+			nodes_restart[0].logger,
 		)
 		.unwrap();
 

lightning/src/ln/channel.rs

@@ -15743,28 +15743,7 @@ pub(crate) fn hold_time_since(send_timestamp: Option<Duration>) -> Option<u32> {
 
 #[cfg(test)]
 mod tests {
-	use std::cmp;
-	use bitcoin::amount::Amount;
-	use bitcoin::constants::ChainHash;
-	use bitcoin::script::{ScriptBuf, Builder};
-	use bitcoin::transaction::{Transaction, TxOut, Version};
-	use bitcoin::opcodes;
-	use bitcoin::network::Network;
-	use crate::ln::onion_utils::INVALID_ONION_BLINDING;
-	use crate::types::payment::{PaymentHash, PaymentPreimage};
-	use crate::ln::channel_keys::{RevocationKey, RevocationBasepoint};
-	use crate::ln::channelmanager::{self, HTLCSource, PaymentId};
-	use crate::ln::channel::InitFeatures;
-	use crate::ln::channel::{AwaitingChannelReadyFlags, Channel, ChannelState, InboundHTLCOutput, OutboundV1Channel, InboundV1Channel, OutboundHTLCOutput, InboundHTLCState, OutboundHTLCState, HTLCCandidate, HTLCInitiator, HTLCUpdateAwaitingACK, commit_tx_fee_sat};
-	use crate::ln::channel::{MAX_FUNDING_SATOSHIS_NO_WUMBO, TOTAL_BITCOIN_SUPPLY_SATOSHIS};
-	use crate::types::features::{ChannelFeatures, ChannelTypeFeatures, NodeFeatures};
-	use crate::ln::msgs;
-	use crate::ln::msgs::{ChannelUpdate, DecodeError, UnsignedChannelUpdate, MAX_VALUE_MSAT};
-	use crate::ln::script::ShutdownScript;
-	use crate::ln::chan_utils::{self, htlc_success_tx_weight, htlc_timeout_tx_weight};
-	use crate::chain::BestBlock;
-	use crate::chain::chaininterface::{FeeEstimator, LowerBoundedFeeEstimator, ConfirmationTarget};
-	use crate::sign::{ChannelSigner, InMemorySigner, EntropySource, SignerProvider};
+	use crate::chain::chaininterface::LowerBoundedFeeEstimator;
 	use crate::chain::transaction::OutPoint;
 	use crate::chain::BestBlock;
 	use crate::ln::chan_utils::{self, commit_tx_fee_sat, ChannelTransactionParameters};
@@ -16290,8 +16269,8 @@ mod tests {
 		outbound_node_config.channel_handshake_config.their_channel_reserve_proportional_millionths = (outbound_selected_channel_reserve_perc * 1_000_000.0) as u32;
 		let mut chan = OutboundV1Channel::<&TestKeysInterface>::new(&&fee_est, &&keys_provider, &&keys_provider, outbound_node_id, &channelmanager::provided_init_features(&outbound_node_config), channel_value_satoshis, 100_000, 42, &outbound_node_config, 0, 42, None, &logger).unwrap();
 
-		let expected_outbound_selected_chan_reserve = cmp::max(outbound_node_config.channel_handshake_config.min_their_channel_reserve_satoshis, (chan.context.channel_value_satoshis as f64 * outbound_selected_channel_reserve_perc) as u64);
-		assert_eq!(chan.context.holder_selected_channel_reserve_satoshis, expected_outbound_selected_chan_reserve);
+		let expected_outbound_selected_chan_reserve = cmp::max(outbound_node_config.channel_handshake_config.min_their_channel_reserve_satoshis, (chan.funding.get_value_satoshis() as f64 * outbound_selected_channel_reserve_perc) as u64);
+		assert_eq!(chan.funding.holder_selected_channel_reserve_satoshis, expected_outbound_selected_chan_reserve);
 
 		let chan_open_channel_msg = chan.get_open_channel(ChainHash::using_genesis_block(network), &&logger).unwrap();
 		let mut inbound_node_config = UserConfig::default();
@@ -16300,7 +16279,7 @@ mod tests {
 		if outbound_selected_channel_reserve_perc + inbound_selected_channel_reserve_perc < 1.0 {
 			let chan_inbound_node = InboundV1Channel::<&TestKeysInterface>::new(&&fee_est, &&keys_provider, &&keys_provider, inbound_node_id, &channelmanager::provided_channel_type_features(&inbound_node_config), &channelmanager::provided_init_features(&outbound_node_config), &chan_open_channel_msg, 7, &inbound_node_config, 0, &&logger, /*is_0conf=*/false).unwrap();
 
-			let expected_inbound_selected_chan_reserve = cmp::max(inbound_node_config.channel_handshake_config.min_their_channel_reserve_satoshis, (chan.context.channel_value_satoshis as f64 * inbound_selected_channel_reserve_perc) as u64);
+			let expected_inbound_selected_chan_reserve = cmp::max(inbound_node_config.channel_handshake_config.min_their_channel_reserve_satoshis, (chan.funding.get_value_satoshis() as f64 * inbound_selected_channel_reserve_perc) as u64);
 
 			assert_eq!(chan_inbound_node.funding.holder_selected_channel_reserve_satoshis, expected_inbound_selected_chan_reserve);
 			assert_eq!(chan_inbound_node.funding.counterparty_selected_channel_reserve_satoshis.unwrap(), expected_outbound_selected_chan_reserve);
@@ -16314,7 +16293,8 @@ mod tests {
 	#[test]
 	#[rustfmt::skip]
 	fn test_configurable_min_channel_reserve() {
-		let fee_est = LowerBoundedFeeEstimator::new(&TestFeeEstimator { fee_est: 15_000 });
+		let inner_fee_est = TestFeeEstimator::new(15_000);
+		let fee_est = LowerBoundedFeeEstimator::new(&inner_fee_est);
 		let logger = test_utils::TestLogger::new();
 		let secp_ctx = Secp256k1::new();
 		let keys_provider = test_utils::TestKeysInterface::new(&[42; 32], Network::Testnet);
@@ -16332,7 +16312,7 @@ mod tests {
 		).unwrap();
 
 		// With 0 minimum and 0 proportional, reserve should be 0 (bypasses dust limit)
-		assert_eq!(chan.context.holder_selected_channel_reserve_satoshis, 0);
+		assert_eq!(chan.funding.holder_selected_channel_reserve_satoshis, 0);
 
 		// Test with custom minimum enforced when proportional is lower
 		config.channel_handshake_config.min_their_channel_reserve_satoshis = 10_000;
@@ -16345,7 +16325,7 @@ mod tests {
 		).unwrap();
 
 		// Proportional would be 1% of 100k = 1000, but minimum is 10000, so 10000 should be used
-		assert_eq!(chan_small.context.holder_selected_channel_reserve_satoshis, 10_000);
+		assert_eq!(chan_small.funding.holder_selected_channel_reserve_satoshis, 10_000);
 
 		// Test that dust limit is still enforced when min_their_channel_reserve_satoshis is non-zero but below dust limit
 		config.channel_handshake_config.min_their_channel_reserve_satoshis = 100; // Below dust limit of 354