Refactor channelmanager tests to test publicly visible apis

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

Author: srikanth-iyengar

lightning/src/ln/anchor_channel_configuration_tests.rs

@@ -1,10 +1,11 @@
 use crate::events::{Event, MessageSendEvent, MessageSendEventsProvider};
-use crate::ln::msgs::ChannelMessageHandler;
+use crate::ln::types::ChannelId;
+use crate::ln::functional_test_utils::*;
 use crate::ln::msgs::ErrorAction;
-use crate::ln::{functional_test_utils::*, ChannelId};
+use crate::ln::msgs::ChannelMessageHandler;
 use crate::prelude::*;
-use crate::util::config::ChannelConfigUpdate;
 use crate::util::errors::APIError;
+use crate::util::config::ChannelConfigUpdate;
 
 #[test]
 fn test_inbound_anchors_manual_acceptance() {

lightning/src/ln/channelmanager.rs

@@ -107,17 +107,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
@@ -2284,15 +2273,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(super) 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(super) 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(super) 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

@@ -1,17 +1,16 @@
 use bitcoin::hashes::Hash;
-use crate::ln::channelmanager::{PaymentId, PaymentSendFailure, RecipientOnionFields};
+use crate::ln::channelmanager::{PaymentId, PaymentSendFailure, RecipientOnionFields, self};
 use crate::ln::functional_test_utils::*;
 use crate::util::errors::APIError;
 use bitcoin::secp256k1::{PublicKey, Secp256k1, SecretKey};
 use crate::events::{Event, MessageSendEvent, MessageSendEventsProvider, ClosureReason};
-use crate::ln::channelmanager;
 use crate::ln::ChannelId;
 use crate::ln::msgs::{self};
 use crate::ln::msgs::ChannelMessageHandler;
 use crate::prelude::*;
 use crate::util::config::ChannelConfig;
 use crate::sign::EntropySource;
-
+use crate::ln::channelmanager::InterceptId;
 
 #[test]
 fn test_notify_limits() {
@@ -246,7 +245,6 @@ fn test_outpoint_to_peer_coverage() {
 	check_closed_event!(nodes[1], 1, ClosureReason::CounterpartyInitiatedCooperativeClosure, [nodes[0].node.get_our_node_id()], 1000000);
 }
 
-
 fn check_not_connected_to_peer_error<T>(res_err: Result<T, APIError>, expected_public_key: PublicKey) {
 	let expected_message = format!("Not connected to node: {}", expected_public_key);
 	check_api_error_message(expected_message, res_err)
@@ -293,7 +291,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);
@@ -336,7 +334,7 @@ fn test_api_calls_with_unavailable_channel() {
 
 	check_channel_unavailable_error(nodes[0].node.force_close_without_broadcasting_txn(&channel_id, &counterparty_node_id), channel_id, counterparty_node_id);
 
-	check_channel_unavailable_error(nodes[0].node.forward_intercepted_htlc(channelmanager::InterceptId([0; 32]), &channel_id, counterparty_node_id, 1_000_000), channel_id, counterparty_node_id);
+	check_channel_unavailable_error(nodes[0].node.forward_intercepted_htlc(InterceptId([0; 32]), &channel_id, counterparty_node_id, 1_000_000), channel_id, counterparty_node_id);
 
 	check_channel_unavailable_error(nodes[0].node.update_channel_config(&counterparty_node_id, &[channel_id], &ChannelConfig::default()), channel_id, counterparty_node_id);
 }
@@ -348,6 +346,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 +390,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 +500,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 +509,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 +528,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 +663,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);
 
@@ -673,8 +673,8 @@ fn test_channel_update_cached() {
 
 	{
 		// 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(), 1);
 	}
 
 	// Test that we do not retrieve the pending broadcast messages when we are not connected to any peer
@@ -704,7 +704,7 @@ 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() {
@@ -132,7 +130,6 @@ fn test_keysend_dup_hash_partial_mpp() {
 	}
 }
 
-
 #[test]
 fn test_keysend_dup_payment_hash() {
 	do_test_keysend_dup_payment_hash(false);
@@ -389,6 +386,11 @@ fn bad_inbound_payment_hash() {
 	let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]);
 	let nodes = create_network(2, &node_cfgs, &node_chanmgrs);
 
+	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,
@@ -399,22 +401,23 @@ fn bad_inbound_payment_hash() {
 	// 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 +434,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 +456,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 +478,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

@@ -83,5 +83,11 @@ mod async_signer_tests;
 mod offers_tests;
 #[allow(dead_code)] // TODO(dual_funding): Exchange for dual_funding cfg
 pub(crate) mod interactivetxs;
+#[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;