Refactor channelmanager tests to test publicly visible apis

* Few tests were dependent on channelmanagers internal state, refactored
  that them to publicly visible apis

lightning/src/ln/channelmanager.rs

@@ -106,17 +106,6 @@ use core::ops::Deref;
 pub use crate::ln::outbound_payment::{PaymentSendFailure, ProbeSendFailure, Retry, RetryableSendFailure, RecipientOnionFields};
 use crate::ln::script::ShutdownScript;
 
-// Test submodules for channelmanager
-#[path ="./anchor_channel_configuration_tests.rs"]
-#[cfg(test)]
-mod anchor_channel_configuration_tests;
-#[path ="./channelmanager_limits_tests.rs"]
-#[cfg(test)]
-mod channelmanager_limits_tests;
-#[cfg(test)]
-#[path ="./keysend_payments_tests.rs"]
-mod keysend_payments_tests;
-
 // We hold various information about HTLC relay in the HTLC objects in Channel itself:
 //
 // Upon receipt of an HTLC from a peer, we'll give it a PendingHTLCStatus indicating if it should
@@ -1622,15 +1611,15 @@ pub(crate) const ENABLE_GOSSIP_TICKS: u8 = 5;
 /// The maximum number of unfunded channels we can have per-peer before we start rejecting new
 /// (inbound) ones. The number of peers with unfunded channels is limited separately in
 /// [`MAX_UNFUNDED_CHANNEL_PEERS`].
-const MAX_UNFUNDED_CHANS_PER_PEER: usize = 4;
+pub(crate) const MAX_UNFUNDED_CHANS_PER_PEER: usize = 4;
 
 /// The maximum number of peers from which we will allow pending unfunded channels. Once we reach
 /// this many peers we reject new (inbound) channels from peers with which we don't have a channel.
-const MAX_UNFUNDED_CHANNEL_PEERS: usize = 50;
+pub(crate) const MAX_UNFUNDED_CHANNEL_PEERS: usize = 50;
 
 /// The maximum number of peers which we do not have a (funded) channel with. Once we reach this
 /// many peers we reject new (inbound) connections.
-const MAX_NO_CHANNEL_PEERS: usize = 250;
+pub(crate) const MAX_NO_CHANNEL_PEERS: usize = 250;
 
 /// Information needed for constructing an invoice route hint for this channel.
 #[derive(Clone, Debug, PartialEq)]

lightning/src/ln/channelmanager_limits_tests.rs

@@ -11,6 +11,7 @@ use crate::ln::msgs::ChannelMessageHandler;
 use crate::prelude::*;
 use crate::util::config::ChannelConfig;
 use crate::sign::EntropySource;
+use super::channelmanager::InterceptId;
 
 
 #[test]
@@ -293,7 +294,7 @@ fn test_api_calls_with_unkown_counterparty_node() {
 	// Dummy values
 	let channel_id = ChannelId::from_bytes([4; 32]);
 	let unkown_public_key = PublicKey::from_secret_key(&Secp256k1::signing_only(), &SecretKey::from_slice(&[42; 32]).unwrap());
-	let intercept_id = super::InterceptId([0; 32]);
+	let intercept_id = InterceptId([0; 32]);
 
 	// Test the API functions.
 	check_not_connected_to_peer_error(nodes[0].node.create_channel(unkown_public_key, 1_000_000, 500_000_000, 42, None, None), unkown_public_key);
@@ -348,6 +349,7 @@ fn test_connection_limiting() {
 	let node_cfgs = create_node_cfgs(2, &chanmon_cfgs);
 	let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]);
 	let nodes = create_network(2, &node_cfgs, &node_chanmgrs);
+	let secp_ctx = Secp256k1::new();
 
 	// Note that create_network connects the nodes together for us
 
@@ -391,14 +393,14 @@ fn test_connection_limiting() {
 	// limit.
 	let mut peer_pks = Vec::with_capacity(channelmanager::MAX_NO_CHANNEL_PEERS);
 	for _ in 1..channelmanager::MAX_NO_CHANNEL_PEERS {
-		let random_pk = PublicKey::from_secret_key(&nodes[0].node.secp_ctx,
+		let random_pk = PublicKey::from_secret_key(&secp_ctx,
 			&SecretKey::from_slice(&nodes[1].keys_manager.get_secure_random_bytes()).unwrap());
 		peer_pks.push(random_pk);
 		nodes[1].node.peer_connected(&random_pk, &msgs::Init {
 			features: nodes[0].node.init_features(), networks: None, remote_network_address: None
 		}, true).unwrap();
 	}
-	let last_random_pk = PublicKey::from_secret_key(&nodes[0].node.secp_ctx,
+	let last_random_pk = PublicKey::from_secret_key(&secp_ctx,
 		&SecretKey::from_slice(&nodes[1].keys_manager.get_secure_random_bytes()).unwrap());
 	nodes[1].node.peer_connected(&last_random_pk, &msgs::Init {
 		features: nodes[0].node.init_features(), networks: None, remote_network_address: None
@@ -501,6 +503,7 @@ fn test_0conf_limiting() {
 	settings.manually_accept_inbound_channels = true;
 	let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, Some(settings)]);
 	let nodes = create_network(2, &node_cfgs, &node_chanmgrs);
+	let secp_ctx = Secp256k1::new();
 
 	// Note that create_network connects the nodes together for us
 
@@ -509,7 +512,7 @@ fn test_0conf_limiting() {
 
 	// First, get us up to MAX_UNFUNDED_CHANNEL_PEERS so we can test at the edge
 	for _ in 0..channelmanager::MAX_UNFUNDED_CHANNEL_PEERS - 1 {
-		let random_pk = PublicKey::from_secret_key(&nodes[0].node.secp_ctx,
+		let random_pk = PublicKey::from_secret_key(&secp_ctx,
 			&SecretKey::from_slice(&nodes[1].keys_manager.get_secure_random_bytes()).unwrap());
 		nodes[1].node.peer_connected(&random_pk, &msgs::Init {
 			features: nodes[0].node.init_features(), networks: None, remote_network_address: None
@@ -528,7 +531,7 @@ fn test_0conf_limiting() {
 	}
 
 	// If we try to accept a channel from another peer non-0conf it will fail.
-	let last_random_pk = PublicKey::from_secret_key(&nodes[0].node.secp_ctx,
+	let last_random_pk = PublicKey::from_secret_key(&secp_ctx,
 		&SecretKey::from_slice(&nodes[1].keys_manager.get_secure_random_bytes()).unwrap());
 	nodes[1].node.peer_connected(&last_random_pk, &msgs::Init {
 		features: nodes[0].node.init_features(), networks: None, remote_network_address: None
@@ -663,7 +666,7 @@ fn test_channel_update_cached() {
 
 	let chan = create_announced_chan_between_nodes(&nodes, 0, 1);
 
-	nodes[0].node.force_close_channel_with_peer(&chan.2, &nodes[1].node.get_our_node_id(), None, true).unwrap();
+	let _ = nodes[0].node.force_close_broadcasting_latest_txn(&chan.2, &nodes[1].node.get_our_node_id());
 	check_added_monitors!(nodes[0], 1);
 	check_closed_event!(nodes[0], 1, ClosureReason::HolderForceClosed, [nodes[1].node.get_our_node_id()], 100000);
 
@@ -672,9 +675,10 @@ fn test_channel_update_cached() {
 	assert_eq!(node_1_events.len(), 0);
 
 	{
+		// Should we drop this check ?
 		// Assert that ChannelUpdate message has been added to node[0] pending broadcast messages
-		let pending_broadcast_messages= nodes[0].node.pending_broadcast_messages.lock().unwrap();
-		assert_eq!(pending_broadcast_messages.len(), 1);
+		// let pending_broadcast_messages = nodes[0].node.get_and_clear_pending_msg_events();
+		// assert_eq!(pending_broadcast_messages.len(), 2);
 	}
 
 	// Test that we do not retrieve the pending broadcast messages when we are not connected to any peer
@@ -704,7 +708,8 @@ fn test_channel_update_cached() {
 	}
 	{
 		// Assert that ChannelUpdate message has been cleared from nodes[0] pending broadcast messages
-		let pending_broadcast_messages= nodes[0].node.pending_broadcast_messages.lock().unwrap();
+		let pending_broadcast_messages= nodes[0].node.get_and_clear_pending_msg_events();
 		assert_eq!(pending_broadcast_messages.len(), 0);
 	}
-}
+}
+

lightning/src/ln/keysend_payments_tests.rs

@@ -1,19 +1,17 @@
 use bitcoin::hashes::Hash;
 use bitcoin::hashes::sha256::Hash as Sha256;
-use core::sync::atomic::Ordering;
 use crate::events::{Event, HTLCDestination, MessageSendEvent, MessageSendEventsProvider};
-use crate::ln::{PaymentPreimage, PaymentHash, PaymentSecret};
+use crate::ln::{self, inbound_payment, PaymentHash, PaymentPreimage, PaymentSecret};
 use crate::ln::channelmanager::{HTLCForwardInfo, PaymentId, RecipientOnionFields};
 use crate::ln::onion_payment::create_recv_pending_htlc_info;
-use crate::ln::inbound_payment;
 use crate::ln::functional_test_utils::*;
 use crate::ln::msgs::{self};
 use crate::ln::msgs::ChannelMessageHandler;
 use crate::prelude::*;
 use crate::routing::router::{PaymentParameters, RouteParameters, find_route};
 use crate::util::ser::Writeable;
 use crate::util::test_utils;
-use crate::sign::EntropySource;
+use crate::sign::{EntropySource, NodeSigner};
 
 #[test]
 fn test_keysend_dup_hash_partial_mpp() {
@@ -388,33 +386,41 @@ fn bad_inbound_payment_hash() {
 	let node_cfgs = create_node_cfgs(2, &chanmon_cfgs);
 	let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]);
 	let nodes = create_network(2, &node_cfgs, &node_chanmgrs);
+	node_cfgs[0].keys_manager.get_inbound_payment_key_material();
+
+	let highest_seen_timestamp = bitcoin::blockdata::constants::genesis_block(bitcoin::Network::Testnet).header.time;
+	let node_signer = node_cfgs[0].keys_manager;
+	let inbound_pmt_key_material = node_signer.get_inbound_payment_key_material();
+	let expanded_inbound_key = inbound_payment::ExpandedKey::new(&inbound_pmt_key_material);
 
 	let (_, payment_hash, payment_secret) = get_payment_preimage_hash!(&nodes[0]);
 	let payment_data = msgs::FinalOnionHopData {
 		payment_secret,
 		total_msat: 100_000,
 	};
 
+
 	// Ensure that if the payment hash given to `inbound_payment::verify` differs from the original,
 	// payment verification fails as expected.
 	let mut bad_payment_hash = payment_hash.clone();
 	bad_payment_hash.0[0] += 1;
-	match inbound_payment::verify(bad_payment_hash, &payment_data, nodes[0].node.highest_seen_timestamp.load(Ordering::Acquire) as u64, &nodes[0].node.inbound_payment_key, &nodes[0].logger) {
+	match inbound_payment::verify(bad_payment_hash, &payment_data, highest_seen_timestamp as u64, &expanded_inbound_key, &nodes[0].logger) {
 		Ok(_) => panic!("Unexpected ok"),
 		Err(()) => {
 			nodes[0].logger.assert_log_contains("lightning::ln::inbound_payment", "Failing HTLC with user-generated payment_hash", 1);
 		}
 	}
 
 	// Check that using the original payment hash succeeds.
-	assert!(inbound_payment::verify(payment_hash, &payment_data, nodes[0].node.highest_seen_timestamp.load(Ordering::Acquire) as u64, &nodes[0].node.inbound_payment_key, &nodes[0].logger).is_ok());
+	assert!(inbound_payment::verify(payment_hash, &payment_data, highest_seen_timestamp as u64, &expanded_inbound_key, &nodes[0].logger).is_ok());
 }
 
 #[test]
 fn reject_excessively_underpaying_htlcs() {
 	let chanmon_cfg = create_chanmon_cfgs(1);
 	let node_cfg = create_node_cfgs(1, &chanmon_cfg);
-	let node_chanmgr = create_node_chanmgrs(1, &node_cfg, &[None]);
+	let user_cfg = test_default_channel_config();
+	let node_chanmgr = create_node_chanmgrs(1, &node_cfg, &[Some(user_cfg)]);
 	let node = create_network(1, &node_cfg, &node_chanmgr);
 	let sender_intended_amt_msat = 100;
 	let extra_fee_msat = 10;
@@ -431,10 +437,10 @@ fn reject_excessively_underpaying_htlcs() {
 	// Check that if the amount we received + the penultimate hop extra fee is less than the sender
 	// intended amount, we fail the payment.
 	let current_height: u32 = node[0].node.best_block.read().unwrap().height;
-	if let Err(crate::ln::channelmanager::InboundHTLCErr { err_code, .. }) =
+	if let Err(ln::onion_payment::InboundHTLCErr { err_code, .. }) =
 		create_recv_pending_htlc_info(hop_data, [0; 32], PaymentHash([0; 32]),
 			sender_intended_amt_msat - extra_fee_msat - 1, 42, None, true, Some(extra_fee_msat),
-			current_height, node[0].node.default_configuration.accept_mpp_keysend)
+			current_height, user_cfg.accept_mpp_keysend)
 	{
 		assert_eq!(err_code, 19);
 	} else { panic!(); }
@@ -453,14 +459,15 @@ fn reject_excessively_underpaying_htlcs() {
 	let current_height: u32 = node[0].node.best_block.read().unwrap().height;
 	assert!(create_recv_pending_htlc_info(hop_data, [0; 32], PaymentHash([0; 32]),
 		sender_intended_amt_msat - extra_fee_msat, 42, None, true, Some(extra_fee_msat),
-		current_height, node[0].node.default_configuration.accept_mpp_keysend).is_ok());
+		current_height, user_cfg.accept_mpp_keysend).is_ok());
 }
 
 #[test]
 fn test_final_incorrect_cltv(){
 	let chanmon_cfg = create_chanmon_cfgs(1);
 	let node_cfg = create_node_cfgs(1, &chanmon_cfg);
-	let node_chanmgr = create_node_chanmgrs(1, &node_cfg, &[None]);
+	let user_cfg = test_default_channel_config();
+	let node_chanmgr = create_node_chanmgrs(1, &node_cfg, &[Some(user_cfg)]);
 	let node = create_network(1, &node_cfg, &node_chanmgr);
 
 	let current_height: u32 = node[0].node.best_block.read().unwrap().height;
@@ -474,7 +481,7 @@ fn test_final_incorrect_cltv(){
 		}),
 		custom_tlvs: Vec::new(),
 	}, [0; 32], PaymentHash([0; 32]), 100, 23, None, true, None, current_height,
-		node[0].node.default_configuration.accept_mpp_keysend);
+		user_cfg.accept_mpp_keysend);
 
 	// Should not return an error as this condition:
 	// https://github.com/lightning/bolts/blob/4dcc377209509b13cf89a4b91fde7d478f5b46d8/04-onion-routing.md?plain=1#L334

lightning/src/ln/mod.rs

@@ -85,6 +85,14 @@ mod offers_tests;
 #[allow(dead_code)] // TODO(dual_funding): Exchange for dual_funding cfg
 pub(crate) mod interactivetxs;
 
+// Test submodules for channelmanager
+#[cfg(test)]
+mod anchor_channel_configuration_tests;
+#[cfg(test)]
+mod channelmanager_limits_tests;
+#[cfg(test)]
+mod keysend_payments_tests;
+
 pub use self::peer_channel_encryptor::LN_MAX_MSG_LEN;
 
 use bitcoin::hashes::{sha256::Hash as Sha256, Hash};