From 08bca058c8f3b6e342d31d32711841ea7225a99f Mon Sep 17 00:00:00 2001 From: amackillop Date: Tue, 5 May 2026 15:29:50 +0100 Subject: [PATCH] Fix tests for LSPS4 and Logger generic additions 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 736d3a87b. 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. --- lightning-background-processor/src/lib.rs | 37 +++++++++++++------ lightning-liquidity/tests/common/mod.rs | 6 ++- .../tests/lsps0_integration_tests.rs | 2 + .../tests/lsps2_integration_tests.rs | 4 ++ .../tests/lsps5_integration_tests.rs | 6 +++ lightning/src/ln/channel.rs | 36 ++++-------------- 6 files changed, 50 insertions(+), 41 deletions(-) diff --git a/lightning-background-processor/src/lib.rs b/lightning-background-processor/src/lib.rs index 3149f692470..a346da5c06d 100644 --- a/lightning-background-processor/src/lib.rs +++ b/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 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> + 'static + Send>> { todo!() } /// # fn list(&self, primary_namespace: &str, secondary_namespace: &str) -> Pin, 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> { Ok(Vec::new()) } +/// # fn write(&self, primary_namespace: &str, secondary_namespace: &str, key: &str, buf: Vec) -> 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> { Ok(Vec::new()) } +/// # } /// # use core::time::Duration; /// # struct DefaultTimeProvider; /// # @@ -786,7 +797,7 @@ use futures_util::{dummy_waker, Joiner, OptionalSelector, Selector, SelectorOutp /// # type P2PGossipSync
    = lightning::routing::gossip::P2PGossipSync, Arc
      , Arc>; /// # type ChannelManager = lightning::ln::channelmanager::SimpleArcChannelManager, B, FE, Logger>; /// # type OnionMessenger = lightning::onion_message::messenger::OnionMessenger, Arc, Arc, Arc>, Arc, Arc, Arc>>, Arc>, lightning::ln::peer_handler::IgnoringMessageHandler, lightning::ln::peer_handler::IgnoringMessageHandler, lightning::ln::peer_handler::IgnoringMessageHandler>; -/// # type LiquidityManager = lightning_liquidity::LiquidityManager, Arc, Arc>, Arc, Arc, Arc, Arc>; +/// # type LiquidityManager = lightning_liquidity::LiquidityManager, Arc, Arc>, Arc, Arc, Arc, Arc, Arc>; /// # type Scorer = RwLock, Arc>>; /// # type PeerManager = lightning::ln::peer_handler::SimpleArcPeerManager, B, FE, Arc
        , Logger, F, StoreSync>; /// # type OutputSweeper = lightning::util::sweep::OutputSweeper, Arc, Arc, Arc, Arc, Arc, Arc>; @@ -1967,6 +1978,7 @@ mod tests { Arc, DefaultTimeProvider, Arc, + Arc, >; 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<_, _, _, _, _, _, _>) diff --git a/lightning-liquidity/tests/common/mod.rs b/lightning-liquidity/tests/common/mod.rs index dea987527ad..08305d236a2 100644 --- a/lightning-liquidity/tests/common/mod.rs +++ b/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, Arc, &'c TestBroadcaster, + &'c TestLogger, >, } @@ -155,6 +158,7 @@ impl<'a, 'b, 'c> LiquidityNode<'a, 'b, 'c> { Arc, Arc, &'c TestBroadcaster, + &'c TestLogger, >, ) -> Self { Self { inner: node, liquidity_manager } diff --git a/lightning-liquidity/tests/lsps0_integration_tests.rs b/lightning-liquidity/tests/lsps0_integration_tests.rs index 423d49785f2..ff41a140902 100644 --- a/lightning-liquidity/tests/lsps0_integration_tests.rs +++ b/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), }; diff --git a/lightning-liquidity/tests/lsps2_integration_tests.rs b/lightning-liquidity/tests/lsps2_integration_tests.rs index 82f93b5990c..f81febdee81 100644 --- a/lightning-liquidity/tests/lsps2_integration_tests.rs +++ b/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(); diff --git a/lightning-liquidity/tests/lsps5_integration_tests.rs b/lightning-liquidity/tests/lsps5_integration_tests.rs index 80707a60774..7932db36464 100644 --- a/lightning-liquidity/tests/lsps5_integration_tests.rs +++ b/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(); diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index ebc9e4e15f9..d7f75e1c4ab 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -15743,28 +15743,7 @@ pub(crate) fn hold_time_since(send_timestamp: Option) -> Option { #[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