From 82fdf0f62d983dfdcbd28f9037f94d70c7d98814 Mon Sep 17 00:00:00 2001 From: Elias Rohrer Date: Sat, 17 Jun 2023 13:16:52 +0200 Subject: [PATCH] Avoid panic when 0conf channel's ann. sigs race on-chain confirmation A channel's `short_channel_id` is currently only set when the funding transaction is confirmed via `transactions_confirmed`, which might be well after the channel initally becomes usable, e.g., in the 0conf case. Previously we would panic due to a reachable `unwrap` when receiving a counterparty's `announcement_signatures` message for a 0conf channel pending confirmation on-chain. Here we fix this bug by avoiding unsafe `unwrap`s and just erroring out and ignoring the announcement_signatures message if the `short_channel_id` hasn't been set yet. --- lightning/src/ln/channel.rs | 10 ++++--- lightning/src/ln/priv_short_conf_tests.rs | 35 +++++++++++++++++++++++ 2 files changed, 41 insertions(+), 4 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index e74dbe42..eb92712c 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -948,9 +948,9 @@ impl ChannelContext { &self.channel_type } - /// Guaranteed to be Some after both ChannelReady messages have been exchanged (and, thus, - /// is_usable() returns true). - /// Allowed in any state (including after shutdown) + /// Gets the channel's `short_channel_id`. + /// + /// Will return `None` if the channel hasn't been confirmed yet. pub fn get_short_channel_id(&self) -> Option { self.short_channel_id } @@ -4832,6 +4832,8 @@ impl Channel { return Err(ChannelError::Ignore("Cannot get a ChannelAnnouncement if the channel is not currently usable".to_owned())); } + let short_channel_id = self.context.get_short_channel_id() + .ok_or(ChannelError::Ignore("Cannot get a ChannelAnnouncement if the channel has not been confirmed yet".to_owned()))?; let node_id = NodeId::from_pubkey(&node_signer.get_node_id(Recipient::Node) .map_err(|_| ChannelError::Ignore("Failed to retrieve own public key".to_owned()))?); let counterparty_node_id = NodeId::from_pubkey(&self.context.get_counterparty_node_id()); @@ -4840,7 +4842,7 @@ impl Channel { let msg = msgs::UnsignedChannelAnnouncement { features: channelmanager::provided_channel_features(&user_config), chain_hash, - short_channel_id: self.context.get_short_channel_id().unwrap(), + short_channel_id, node_id_1: if were_node_one { node_id } else { counterparty_node_id }, node_id_2: if were_node_one { counterparty_node_id } else { node_id }, bitcoin_key_1: NodeId::from_pubkey(if were_node_one { &self.context.get_holder_pubkeys().funding_pubkey } else { self.context.counterparty_funding_pubkey() }), diff --git a/lightning/src/ln/priv_short_conf_tests.rs b/lightning/src/ln/priv_short_conf_tests.rs index 8d3fbf0e..3a6a8b92 100644 --- a/lightning/src/ln/priv_short_conf_tests.rs +++ b/lightning/src/ln/priv_short_conf_tests.rs @@ -1007,3 +1007,38 @@ fn test_connect_before_funding() { connect_blocks(&nodes[0], 1); connect_blocks(&nodes[1], 1); } + +#[test] +fn test_0conf_ann_sigs_racing_conf() { + // Previously we had a bug where we'd panic when receiving a counterparty's + // announcement_signatures message for a 0conf channel pending confirmation on-chain. Here we + // check that we just error out, ignore the announcement_signatures message, and proceed + // instead. + let chanmon_cfgs = create_chanmon_cfgs(2); + let node_cfgs = create_node_cfgs(2, &chanmon_cfgs); + let mut chan_config = test_default_channel_config(); + chan_config.manually_accept_inbound_channels = true; + + let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, Some(chan_config)]); + let nodes = create_network(2, &node_cfgs, &node_chanmgrs); + + // This is the default but we force it on anyway + chan_config.channel_handshake_config.announced_channel = true; + let (tx, ..) = open_zero_conf_channel(&nodes[0], &nodes[1], Some(chan_config)); + + // We can use the channel immediately, but we can't announce it until we get 6+ confirmations + send_payment(&nodes[0], &[&nodes[1]], 100_000); + + let scid = confirm_transaction(&nodes[0], &tx); + let as_announcement_sigs = get_event_msg!(nodes[0], MessageSendEvent::SendAnnouncementSignatures, nodes[1].node.get_our_node_id()); + + // Handling the announcement_signatures prior to the first confirmation would panic before. + nodes[1].node.handle_announcement_signatures(&nodes[0].node.get_our_node_id(), &as_announcement_sigs); + + assert_eq!(confirm_transaction(&nodes[1], &tx), scid); + let bs_announcement_sigs = get_event_msg!(nodes[1], MessageSendEvent::SendAnnouncementSignatures, nodes[0].node.get_our_node_id()); + + nodes[0].node.handle_announcement_signatures(&nodes[1].node.get_our_node_id(), &bs_announcement_sigs); + let as_announcement = nodes[0].node.get_and_clear_pending_msg_events(); + assert_eq!(as_announcement.len(), 1); +} -- 2.30.2