Refactor async sign counterparty commitment tests

Author: alecchendev

lightning/src/ln/async_signer_tests.rs

@@ -10,19 +10,54 @@
 //! Tests for asynchronous signing. These tests verify that the channel state machine behaves
 //! properly with a signer implementation that asynchronously derives signatures.
 
+use bitcoin::secp256k1::PublicKey;
 use bitcoin::{Transaction, TxOut, TxIn, Amount};
 use bitcoin::blockdata::locktime::absolute::LockTime;
 use bitcoin::transaction::Version;
 
 use crate::chain::channelmonitor::LATENCY_GRACE_PERIOD_BLOCKS;
 use crate::events::bump_transaction::WalletSource;
 use crate::events::{Event, MessageSendEvent, MessageSendEventsProvider, ClosureReason};
+use crate::ln::ChannelId;
 use crate::ln::functional_test_utils::*;
 use crate::ln::msgs::ChannelMessageHandler;
 use crate::ln::channelmanager::{PaymentId, RecipientOnionFields};
+use crate::util::test_channel_signer::ops;
+
+/// Helper to run operations with a simulated asynchronous signer.
+///
+/// Disables the signer for the specified channel and then runs `do_fn`, then re-enables the signer
+/// and calls `signer_unblocked`.
+#[cfg(test)]
+pub fn with_async_signer<DoFn, T>(node: &Node, peer_id: &PublicKey, channel_id: &ChannelId, masks: Vec<u32>, do_fn: DoFn) -> T
+	where DoFn: Fn() -> T
+{
+	let mask = masks.iter().fold(0, |acc, m| (acc | m));
+	eprintln!("disabling {}", ops::string_from(mask));
+	node.set_channel_signer_ops_available(peer_id, channel_id, mask, false);
+	let res = do_fn();
+
+	// Recompute the channel ID just in case the original ID was temporary.
+	let new_channel_id = {
+		let channels = node.node.list_channels();
+		assert_eq!(channels.len(), 1, "expected one channel, not {}", channels.len());
+		channels[0].channel_id
+	};
+
+	for mask in masks {
+		eprintln!("enabling {} and calling signer_unblocked", ops::string_from(mask));
+		node.set_channel_signer_ops_available(peer_id, &new_channel_id, mask, true);
+		node.node.signer_unblocked(Some((*peer_id, new_channel_id)));
+	}
+	res
+}
 
 #[test]
 fn test_async_commitment_signature_for_funding_created() {
+	do_test_async_funding_created(vec![ops::SIGN_COUNTERPARTY_COMMITMENT]);
+}
+
+fn do_test_async_funding_created(masks: Vec<u32>) {
 	// Simulate acquiring the signature for `funding_created` asynchronously.
 	let chanmon_cfgs = create_chanmon_cfgs(2);
 	let node_cfgs = create_node_cfgs(2, &chanmon_cfgs);
@@ -43,23 +78,15 @@ fn test_async_commitment_signature_for_funding_created() {
 	// But! Let's make node[0]'s signer be unavailable: we should *not* broadcast a funding_created
 	// message...
 	let (temporary_channel_id, tx, _) = create_funding_transaction(&nodes[0], &nodes[1].node.get_our_node_id(), 100000, 42);
-	nodes[0].set_channel_signer_available(&nodes[1].node.get_our_node_id(), &temporary_channel_id, false);
-	nodes[0].node.funding_transaction_generated(&temporary_channel_id, &nodes[1].node.get_our_node_id(), tx.clone()).unwrap();
-	check_added_monitors(&nodes[0], 0);
-
-	assert!(nodes[0].node.get_and_clear_pending_msg_events().is_empty());
-
-	// Now re-enable the signer and simulate a retry. The temporary_channel_id won't work anymore so
-	// we have to dig out the real channel ID.
-	let chan_id = {
-		let channels = nodes[0].node.list_channels();
-		assert_eq!(channels.len(), 1, "expected one channel, not {}", channels.len());
-		channels[0].channel_id
-	};
-
-	nodes[0].set_channel_signer_available(&nodes[1].node.get_our_node_id(), &chan_id, true);
-	nodes[0].node.signer_unblocked(Some((nodes[1].node.get_our_node_id(), chan_id)));
-
+	with_async_signer(&nodes[0], &nodes[1].node.get_our_node_id(), &temporary_channel_id, masks, || {
+		nodes[0].node.funding_transaction_generated(&temporary_channel_id, &nodes[1].node.get_our_node_id(), tx.clone()).unwrap();
+		check_added_monitors(&nodes[0], 0);
+		let events = nodes[0].node.get_and_clear_pending_msg_events();
+		assert!(events.is_empty(), "expected no message, got {:?}", events);
+	});
+
+	//// Now that we've re-enabled the signer, simulate a retry. The temporary_channel_id won't work anymore so
+	//// we have to dig out the real channel ID.
 	let mut funding_created_msg = get_event_msg!(nodes[0], MessageSendEvent::SendFundingCreated, nodes[1].node.get_our_node_id());
 	nodes[1].node.handle_funding_created(&nodes[0].node.get_our_node_id(), &funding_created_msg);
 	check_added_monitors(&nodes[1], 1);
@@ -74,6 +101,10 @@ fn test_async_commitment_signature_for_funding_created() {
 
 #[test]
 fn test_async_commitment_signature_for_funding_signed() {
+	do_test_async_funding_signed(vec![ops::SIGN_COUNTERPARTY_COMMITMENT]);
+}
+
+fn do_test_async_funding_signed(masks: Vec<u32>) {
 	// Simulate acquiring the signature for `funding_signed` asynchronously.
 	let chanmon_cfgs = create_chanmon_cfgs(2);
 	let node_cfgs = create_node_cfgs(2, &chanmon_cfgs);
@@ -98,22 +129,14 @@ fn test_async_commitment_signature_for_funding_signed() {
 
 	// Now let's make node[1]'s signer be unavailable while handling the `funding_created`. It should
 	// *not* broadcast a `funding_signed`...
-	nodes[1].set_channel_signer_available(&nodes[0].node.get_our_node_id(), &temporary_channel_id, false);
-	nodes[1].node.handle_funding_created(&nodes[0].node.get_our_node_id(), &funding_created_msg);
-	check_added_monitors(&nodes[1], 1);
-
-	assert!(nodes[1].node.get_and_clear_pending_msg_events().is_empty());
+	with_async_signer(&nodes[1], &nodes[0].node.get_our_node_id(), &temporary_channel_id, masks, || {
+		nodes[1].node.handle_funding_created(&nodes[0].node.get_our_node_id(), &funding_created_msg);
+		check_added_monitors(&nodes[1], 1);
+		assert!(nodes[1].node.get_and_clear_pending_msg_events().is_empty());
+	});
 
-	// Now re-enable the signer and simulate a retry. The temporary_channel_id won't work anymore so
+	// Now that we've re-enabled the signer, simulate a retry. The temporary_channel_id won't work anymore so
 	// we have to dig out the real channel ID.
-	let chan_id = {
-		let channels = nodes[0].node.list_channels();
-		assert_eq!(channels.len(), 1, "expected one channel, not {}", channels.len());
-		channels[0].channel_id
-	};
-	nodes[1].set_channel_signer_available(&nodes[0].node.get_our_node_id(), &chan_id, true);
-	nodes[1].node.signer_unblocked(Some((nodes[0].node.get_our_node_id(), chan_id)));
-
 	expect_channel_pending_event(&nodes[1], &nodes[0].node.get_our_node_id());
 
 	// nodes[0] <-- funding_signed --- nodes[1]
@@ -125,6 +148,10 @@ fn test_async_commitment_signature_for_funding_signed() {
 
 #[test]
 fn test_async_commitment_signature_for_commitment_signed() {
+	do_test_async_commitment_signed(vec![ops::SIGN_COUNTERPARTY_COMMITMENT]);
+}
+
+fn do_test_async_commitment_signed(masks: Vec<u32>) {
 	let chanmon_cfgs = create_chanmon_cfgs(2);
 	let node_cfgs = create_node_cfgs(2, &chanmon_cfgs);
 	let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]);
@@ -150,18 +177,15 @@ fn test_async_commitment_signature_for_commitment_signed() {
 
 	dst.node.handle_update_add_htlc(&src.node.get_our_node_id(), &payment_event.msgs[0]);
 
-	// Mark dst's signer as unavailable and handle src's commitment_signed: while dst won't yet have a
-	// `commitment_signed` of its own to offer, it should publish a `revoke_and_ack`.
-	dst.set_channel_signer_available(&src.node.get_our_node_id(), &chan_id, false);
-	dst.node.handle_commitment_signed(&src.node.get_our_node_id(), &payment_event.commitment_msg);
-	check_added_monitors(dst, 1);
-
-	get_event_msg!(dst, MessageSendEvent::SendRevokeAndACK, src.node.get_our_node_id());
-
-	// Mark dst's signer as available and retry: we now expect to see dst's `commitment_signed`.
-	dst.set_channel_signer_available(&src.node.get_our_node_id(), &chan_id, true);
-	dst.node.signer_unblocked(Some((src.node.get_our_node_id(), chan_id)));
+	// Mark dst's signer as unavailable and handle src's commitment_signed. If dst's signer is
+	// offline, it oughtn't yet respond with any updates.
+	with_async_signer(dst, &src.node.get_our_node_id(), &chan_id, masks, &|| {
+		dst.node.handle_commitment_signed(&src.node.get_our_node_id(), &payment_event.commitment_msg);
+		check_added_monitors(dst, 1);
+		get_event_msg!(dst, MessageSendEvent::SendRevokeAndACK, src.node.get_our_node_id());
+	});
 
+	// Now that we marked dst's signer as available, retry: we now expect to see dst's `commitment_signed`.
 	let events = dst.node.get_and_clear_pending_msg_events();
 	assert_eq!(events.len(), 1, "expected one message, got {}", events.len());
 	if let MessageSendEvent::UpdateHTLCs { ref node_id, .. } = events[0] {
@@ -173,6 +197,10 @@ fn test_async_commitment_signature_for_commitment_signed() {
 
 #[test]
 fn test_async_commitment_signature_for_funding_signed_0conf() {
+	do_test_async_funding_signed_0conf(vec![ops::SIGN_COUNTERPARTY_COMMITMENT]);
+}
+
+fn do_test_async_funding_signed_0conf(masks: Vec<u32>) {
 	// Simulate acquiring the signature for `funding_signed` asynchronously for a zero-conf channel.
 	let mut manually_accept_config = test_default_channel_config();
 	manually_accept_config.manually_accept_inbound_channels = true;
@@ -215,24 +243,13 @@ fn test_async_commitment_signature_for_funding_signed_0conf() {
 
 	// Now let's make node[1]'s signer be unavailable while handling the `funding_created`. It should
 	// *not* broadcast a `funding_signed`...
-	nodes[1].set_channel_signer_available(&nodes[0].node.get_our_node_id(), &temporary_channel_id, false);
-	nodes[1].node.handle_funding_created(&nodes[0].node.get_our_node_id(), &funding_created_msg);
-	check_added_monitors(&nodes[1], 1);
-
-	assert!(nodes[1].node.get_and_clear_pending_msg_events().is_empty());
-
-	// Now re-enable the signer and simulate a retry. The temporary_channel_id won't work anymore so
-	// we have to dig out the real channel ID.
-	let chan_id = {
-		let channels = nodes[0].node.list_channels();
-		assert_eq!(channels.len(), 1, "expected one channel, not {}", channels.len());
-		channels[0].channel_id
-	};
+	with_async_signer(&nodes[1], &nodes[0].node.get_our_node_id(), &temporary_channel_id, masks, || {
+		nodes[1].node.handle_funding_created(&nodes[0].node.get_our_node_id(), &funding_created_msg);
+		check_added_monitors(&nodes[1], 1);
+		assert!(nodes[1].node.get_and_clear_pending_msg_events().is_empty());
+	});
 
 	// At this point, we basically expect the channel to open like a normal zero-conf channel.
-	nodes[1].set_channel_signer_available(&nodes[0].node.get_our_node_id(), &chan_id, true);
-	nodes[1].node.signer_unblocked(Some((nodes[0].node.get_our_node_id(), chan_id)));
-
 	let (funding_signed, channel_ready_1) = {
 		let events = nodes[1].node.get_and_clear_pending_msg_events();
 		assert_eq!(events.len(), 2);
@@ -299,24 +316,22 @@ fn test_async_commitment_signature_for_peer_disconnect() {
 
 	// Mark dst's signer as unavailable and handle src's commitment_signed: while dst won't yet have a
 	// `commitment_signed` of its own to offer, it should publish a `revoke_and_ack`.
-	dst.set_channel_signer_available(&src.node.get_our_node_id(), &chan_id, false);
-	dst.node.handle_commitment_signed(&src.node.get_our_node_id(), &payment_event.commitment_msg);
-	check_added_monitors(dst, 1);
-
-	get_event_msg!(dst, MessageSendEvent::SendRevokeAndACK, src.node.get_our_node_id());
-
-	// Now disconnect and reconnect the peers.
-	src.node.peer_disconnected(&dst.node.get_our_node_id());
-	dst.node.peer_disconnected(&src.node.get_our_node_id());
-	let mut reconnect_args = ReconnectArgs::new(&nodes[0], &nodes[1]);
-	reconnect_args.send_channel_ready = (false, false);
-	reconnect_args.pending_raa = (true, false);
-	reconnect_nodes(reconnect_args);
-
-	// Mark dst's signer as available and retry: we now expect to see dst's `commitment_signed`.
-	dst.set_channel_signer_available(&src.node.get_our_node_id(), &chan_id, true);
-	dst.node.signer_unblocked(Some((src.node.get_our_node_id(), chan_id)));
-
+	with_async_signer(dst, &src.node.get_our_node_id(), &chan_id, vec![ops::SIGN_COUNTERPARTY_COMMITMENT], &|| {
+		dst.node.handle_commitment_signed(&src.node.get_our_node_id(), &payment_event.commitment_msg);
+		check_added_monitors(dst, 1);
+
+		get_event_msg!(dst, MessageSendEvent::SendRevokeAndACK, src.node.get_our_node_id());
+
+		// Now disconnect and reconnect the peers.
+		src.node.peer_disconnected(&dst.node.get_our_node_id());
+		dst.node.peer_disconnected(&src.node.get_our_node_id());
+		let mut reconnect_args = ReconnectArgs::new(&nodes[0], &nodes[1]);
+		reconnect_args.send_channel_ready = (false, false);
+		reconnect_args.pending_raa = (true, false);
+		reconnect_nodes(reconnect_args);
+	});
+
+	// Now that dst's signer is available, retry: we now expect to see dst's `commitment_signed`.
 	{
 		let events = dst.node.get_and_clear_pending_msg_events();
 		assert_eq!(events.len(), 1, "expected one message, got {}", events.len());

lightning/src/util/test_channel_signer.rs

@@ -233,7 +233,7 @@ impl EcdsaChannelSigner for TestChannelSigner {
 		self.verify_counterparty_commitment_tx(commitment_tx, secp_ctx);
 
 		{
-			if !*self.available.lock().unwrap() {
+			if !self.is_signer_available(ops::SIGN_COUNTERPARTY_COMMITMENT) {
 				return Err(());
 			}
 			let mut state = self.state.lock().unwrap();