From 4ef8d92a94f857520e151752f808ffece2e952e1 Mon Sep 17 00:00:00 2001 From: Chris Waterson Date: Sun, 24 Sep 2023 07:34:08 -0700 Subject: [PATCH] Fix inbound zero-conf When we receive an inbound zero-conf channel, we need to defer sending the `channel_ready` message until *after* we've sent the `funding_signed` message. We won't be able to produce the `funding_signed` message until the signer has produced the counterparty commitment signature. --- lightning/src/ln/async_signer_tests.rs | 105 ++++++++++++++++++++++++- lightning/src/ln/channel.rs | 10 ++- lightning/src/ln/channelmanager.rs | 5 +- 3 files changed, 116 insertions(+), 4 deletions(-) diff --git a/lightning/src/ln/async_signer_tests.rs b/lightning/src/ln/async_signer_tests.rs index 64a68e1e7..a0045b771 100644 --- a/lightning/src/ln/async_signer_tests.rs +++ b/lightning/src/ln/async_signer_tests.rs @@ -10,7 +10,7 @@ //! Tests for asynchronous signing. These tests verify that the channel state machine behaves //! properly with a signer implementation that asynchronously derives signatures. -use crate::events::{MessageSendEvent, MessageSendEventsProvider}; +use crate::events::{Event, MessageSendEvent, MessageSendEventsProvider}; use crate::ln::functional_test_utils::*; use crate::ln::msgs::ChannelMessageHandler; use crate::ln::channelmanager::{PaymentId, RecipientOnionFields}; @@ -183,6 +183,109 @@ fn test_async_commitment_signature_for_commitment_signed() { }; } +#[test] +fn test_async_commitment_signature_for_funding_signed_0conf() { + // 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; + + 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, Some(manually_accept_config)]); + let nodes = create_network(2, &node_cfgs, &node_chanmgrs); + + // nodes[0] --- open_channel --> nodes[1] + nodes[0].node.create_channel(nodes[1].node.get_our_node_id(), 100000, 10001, 42, None).unwrap(); + let open_channel = get_event_msg!(nodes[0], MessageSendEvent::SendOpenChannel, nodes[1].node.get_our_node_id()); + + nodes[1].node.handle_open_channel(&nodes[0].node.get_our_node_id(), &open_channel); + + { + let events = nodes[1].node.get_and_clear_pending_events(); + assert_eq!(events.len(), 1, "Expected one event, got {}", events.len()); + match &events[0] { + Event::OpenChannelRequest { temporary_channel_id, .. } => { + nodes[1].node.accept_inbound_channel_from_trusted_peer_0conf( + temporary_channel_id, &nodes[0].node.get_our_node_id(), 0) + .expect("Unable to accept inbound zero-conf channel"); + }, + ev => panic!("Expected OpenChannelRequest, not {:?}", ev) + } + } + + // nodes[0] <-- accept_channel --- nodes[1] + let accept_channel = get_event_msg!(nodes[1], MessageSendEvent::SendAcceptChannel, nodes[0].node.get_our_node_id()); + assert_eq!(accept_channel.minimum_depth, 0, "Expected minimum depth of 0"); + nodes[0].node.handle_accept_channel(&nodes[1].node.get_our_node_id(), &accept_channel); + + // nodes[0] --- funding_created --> nodes[1] + let (temporary_channel_id, tx, _) = create_funding_transaction(&nodes[0], &nodes[1].node.get_our_node_id(), 100000, 42); + 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 mut funding_created_msg = get_event_msg!(nodes[0], MessageSendEvent::SendFundingCreated, nodes[1].node.get_our_node_id()); + + // 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); + + { + let events = nodes[1].node.get_and_clear_pending_msg_events(); + let n = events.len(); + assert_eq!(n, 0, "expected no events generated from nodes[1], found {}", n); + } + + // 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 + }; + + // 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); + let funding_signed = match &events[0] { + MessageSendEvent::SendFundingSigned { msg, .. } => msg.clone(), + ev => panic!("Expected SendFundingSigned, not {:?}", ev) + }; + let channel_ready = match &events[1] { + MessageSendEvent::SendChannelReady { msg, .. } => msg.clone(), + ev => panic!("Expected SendChannelReady, not {:?}", ev) + }; + (funding_signed, channel_ready) + }; + + nodes[0].node.handle_funding_signed(&nodes[1].node.get_our_node_id(), &funding_signed); + expect_channel_pending_event(&nodes[0], &nodes[1].node.get_our_node_id()); + expect_channel_pending_event(&nodes[1], &nodes[0].node.get_our_node_id()); + check_added_monitors(&nodes[0], 1); + + let channel_ready_0 = get_event_msg!(nodes[0], MessageSendEvent::SendChannelReady, nodes[1].node.get_our_node_id()); + + nodes[0].node.handle_channel_ready(&nodes[1].node.get_our_node_id(), &channel_ready_1); + expect_channel_ready_event(&nodes[0], &nodes[1].node.get_our_node_id()); + + nodes[1].node.handle_channel_ready(&nodes[0].node.get_our_node_id(), &channel_ready_0); + expect_channel_ready_event(&nodes[1], &nodes[0].node.get_our_node_id()); + + let channel_update_0 = get_event_msg!(nodes[0], MessageSendEvent::SendChannelUpdate, nodes[1].node.get_our_node_id()); + let channel_update_1 = get_event_msg!(nodes[1], MessageSendEvent::SendChannelUpdate, nodes[0].node.get_our_node_id()); + + nodes[0].node.handle_channel_update(&nodes[1].node.get_our_node_id(), &channel_update_1); + nodes[1].node.handle_channel_update(&nodes[0].node.get_our_node_id(), &channel_update_0); + + assert_eq!(nodes[0].node.list_usable_channels().len(), 1); + assert_eq!(nodes[1].node.list_usable_channels().len(), 1); +} + #[test] fn test_async_commitment_signature_for_peer_disconnect() { let chanmon_cfgs = create_chanmon_cfgs(2); diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 4c8a99028..ee58674d0 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -539,6 +539,7 @@ pub(super) struct SignerResumeUpdates { pub commitment_update: Option, pub funding_signed: Option, pub funding_created: Option, + pub channel_ready: Option, } /// The return value of `channel_reestablish` @@ -3987,6 +3988,9 @@ impl Channel where let funding_signed = if self.context.signer_pending_funding && !self.context.is_outbound() { self.context.get_funding_signed_msg(logger).1 } else { None }; + let channel_ready = if funding_signed.is_some() { + self.check_get_channel_ready(0) + } else { None }; let funding_created = if self.context.signer_pending_funding && self.context.is_outbound() { self.context.get_funding_created_msg(logger) } else { None }; @@ -3994,6 +3998,7 @@ impl Channel where commitment_update, funding_signed, funding_created, + channel_ready, } } @@ -6817,7 +6822,8 @@ impl InboundV1Channel where SP::Target: SignerProvider { counterparty_initial_commitment_tx.to_broadcaster_value_sat(), counterparty_initial_commitment_tx.to_countersignatory_value_sat(), logger); - log_info!(logger, "Generated funding_signed for peer for channel {}", &self.context.channel_id()); + log_info!(logger, "{} funding_signed for peer for channel {}", + if funding_signed.is_some() { "Generated" } else { "Waiting for signature on" }, &self.context.channel_id()); // Promote the channel to a full-fledged one now that we have updated the state and have a // `ChannelMonitor`. @@ -6825,7 +6831,7 @@ impl InboundV1Channel where SP::Target: SignerProvider { context: self.context, }; - let need_channel_ready = channel.check_get_channel_ready(0).is_some(); + let need_channel_ready = funding_signed.is_some() && channel.check_get_channel_ready(0).is_some(); channel.monitor_updating_paused(false, false, need_channel_ready, Vec::new(), Vec::new(), Vec::new()); Ok((channel, funding_signed, channel_monitor)) diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 5e1604b3f..3ca3af7c7 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -7244,9 +7244,9 @@ where let _persistence_guard = PersistenceNotifierGuard::notify_on_drop(self); let unblock_chan = |phase: &mut ChannelPhase, pending_msg_events: &mut Vec| { + let node_id = phase.context().get_counterparty_node_id(); if let ChannelPhase::Funded(chan) = phase { let msgs = chan.signer_maybe_unblocked(&self.logger); - let node_id = phase.context().get_counterparty_node_id(); if let Some(updates) = msgs.commitment_update { pending_msg_events.push(events::MessageSendEvent::UpdateHTLCs { node_id, @@ -7265,6 +7265,9 @@ where msg, }); } + if let Some(msg) = msgs.channel_ready { + send_channel_ready!(self, pending_msg_events, chan, msg); + } } }; -- 2.39.5