Do not panic if a peer learns our funding info before we fund
authorMatt Corallo <git@bluematt.me>
Tue, 26 Dec 2023 18:16:51 +0000 (18:16 +0000)
committerMatt Corallo <git@bluematt.me>
Fri, 29 Dec 2023 06:27:31 +0000 (06:27 +0000)
We'd previously assumed that LDK would receive
`funding_transaction_generated` prior to our peer learning the txid
and panicked if the peer tried to open a redundant channel to us
with the same funding outpoint.

While this assumption is generally safe, some users may have
out-of-band protocols where they notify their LSP about a funding
outpoint first, or this may be violated in the future with
collaborative transaction construction protocols, i.e. the upcoming
dual-funding protocol.

lightning/src/ln/channelmanager.rs
lightning/src/ln/functional_test_utils.rs
lightning/src/ln/functional_tests.rs

index 02852877d93acafe214f7398aa48f3addd77055c..7fe99607528f1bef92007199dc4c04417161c167 100644 (file)
@@ -3738,7 +3738,7 @@ where
                let mut peer_state_lock = peer_state_mutex.lock().unwrap();
                let peer_state = &mut *peer_state_lock;
                let funding_txo;
-               let (chan, msg_opt) = match peer_state.channel_by_id.remove(temporary_channel_id) {
+               let (mut chan, msg_opt) = match peer_state.channel_by_id.remove(temporary_channel_id) {
                        Some(ChannelPhase::UnfundedOutboundV1(mut chan)) => {
                                funding_txo = find_funding_output(&chan, &funding_transaction)?;
 
@@ -3788,8 +3788,20 @@ where
                        },
                        hash_map::Entry::Vacant(e) => {
                                let mut outpoint_to_peer = self.outpoint_to_peer.lock().unwrap();
-                               if outpoint_to_peer.insert(funding_txo, chan.context.get_counterparty_node_id()).is_some() {
-                                       panic!("outpoint_to_peer map already contained funding outpoint, which shouldn't be possible");
+                               match outpoint_to_peer.entry(funding_txo) {
+                                       hash_map::Entry::Vacant(e) => { e.insert(chan.context.get_counterparty_node_id()); },
+                                       hash_map::Entry::Occupied(o) => {
+                                               let err = format!(
+                                                       "An existing channel using outpoint {} is open with peer {}",
+                                                       funding_txo, o.get()
+                                               );
+                                               mem::drop(outpoint_to_peer);
+                                               mem::drop(peer_state_lock);
+                                               mem::drop(per_peer_state);
+                                               let reason = ClosureReason::ProcessingError { err: err.clone() };
+                                               self.finish_close_channel(chan.context.force_shutdown(true, reason));
+                                               return Err(APIError::ChannelUnavailable { err });
+                                       }
                                }
                                e.insert(ChannelPhase::UnfundedOutboundV1(chan));
                        }
index a2d9631716c4c23098135a8f95ee37a1fc5a6949..2adfe2f9379c04c6352a709ff2471232f87d6141 100644 (file)
@@ -1218,7 +1218,7 @@ pub fn open_zero_conf_channel<'a, 'b, 'c, 'd>(initiator: &'a Node<'b, 'c, 'd>, r
        (tx, as_channel_ready.channel_id)
 }
 
-pub fn create_chan_between_nodes_with_value_init<'a, 'b, 'c>(node_a: &Node<'a, 'b, 'c>, node_b: &Node<'a, 'b, 'c>, channel_value: u64, push_msat: u64) -> Transaction {
+pub fn exchange_open_accept_chan<'a, 'b, 'c>(node_a: &Node<'a, 'b, 'c>, node_b: &Node<'a, 'b, 'c>, channel_value: u64, push_msat: u64) -> ChannelId {
        let create_chan_id = node_a.node.create_channel(node_b.node.get_our_node_id(), channel_value, push_msat, 42, None, None).unwrap();
        let open_channel_msg = get_event_msg!(node_a, MessageSendEvent::SendOpenChannel, node_b.node.get_our_node_id());
        assert_eq!(open_channel_msg.temporary_channel_id, create_chan_id);
@@ -1238,6 +1238,11 @@ pub fn create_chan_between_nodes_with_value_init<'a, 'b, 'c>(node_a: &Node<'a, '
        node_a.node.handle_accept_channel(&node_b.node.get_our_node_id(), &accept_channel_msg);
        assert_ne!(node_b.node.list_channels().iter().find(|channel| channel.channel_id == create_chan_id).unwrap().user_channel_id, 0);
 
+       create_chan_id
+}
+
+pub fn create_chan_between_nodes_with_value_init<'a, 'b, 'c>(node_a: &Node<'a, 'b, 'c>, node_b: &Node<'a, 'b, 'c>, channel_value: u64, push_msat: u64) -> Transaction {
+       let create_chan_id = exchange_open_accept_chan(node_a, node_b, channel_value, push_msat);
        sign_funding_transaction(node_a, node_b, channel_value, create_chan_id)
 }
 
index f86eb53e283501dbaeaeae3cc83e79ba1d679aa2..9777ad75d2337edfa73267a29a1e398cf89284c5 100644 (file)
@@ -8977,6 +8977,61 @@ fn test_duplicate_temporary_channel_id_from_different_peers() {
        }
 }
 
+#[test]
+fn test_peer_funding_sidechannel() {
+       // Test that if a peer somehow learns which txid we'll use for our channel funding before we
+       // receive `funding_transaction_generated` the peer cannot cause us to crash. We'd previously
+       // assumed that LDK would receive `funding_transaction_generated` prior to our peer learning
+       // the txid and panicked if the peer tried to open a redundant channel to us with the same
+       // funding outpoint.
+       //
+       // While this assumption is generally safe, some users may have out-of-band protocols where
+       // they notify their LSP about a funding outpoint first, or this may be violated in the future
+       // with collaborative transaction construction protocols, i.e. dual-funding.
+       let chanmon_cfgs = create_chanmon_cfgs(3);
+       let node_cfgs = create_node_cfgs(3, &chanmon_cfgs);
+       let node_chanmgrs = create_node_chanmgrs(3, &node_cfgs, &[None, None, None]);
+       let nodes = create_network(3, &node_cfgs, &node_chanmgrs);
+
+       let temp_chan_id_ab = exchange_open_accept_chan(&nodes[0], &nodes[1], 1_000_000, 0);
+       let temp_chan_id_ca = exchange_open_accept_chan(&nodes[2], &nodes[0], 1_000_000, 0);
+
+       let (_, tx, funding_output) =
+               create_funding_transaction(&nodes[0], &nodes[1].node.get_our_node_id(), 1_000_000, 42);
+
+       let cs_funding_events = nodes[2].node.get_and_clear_pending_events();
+       assert_eq!(cs_funding_events.len(), 1);
+       match cs_funding_events[0] {
+               Event::FundingGenerationReady { .. } => {}
+               _ => panic!("Unexpected event {:?}", cs_funding_events),
+       }
+
+       nodes[2].node.funding_transaction_generated_unchecked(&temp_chan_id_ca, &nodes[0].node.get_our_node_id(), tx.clone(), funding_output.index).unwrap();
+       let funding_created_msg = get_event_msg!(nodes[2], MessageSendEvent::SendFundingCreated, nodes[0].node.get_our_node_id());
+       nodes[0].node.handle_funding_created(&nodes[2].node.get_our_node_id(), &funding_created_msg);
+       get_event_msg!(nodes[0], MessageSendEvent::SendFundingSigned, nodes[2].node.get_our_node_id());
+       expect_channel_pending_event(&nodes[0], &nodes[2].node.get_our_node_id());
+       check_added_monitors!(nodes[0], 1);
+
+       let res = nodes[0].node.funding_transaction_generated(&temp_chan_id_ab, &nodes[1].node.get_our_node_id(), tx.clone());
+       let err_msg = format!("{:?}", res.unwrap_err());
+       assert!(err_msg.contains("An existing channel using outpoint "));
+       assert!(err_msg.contains(" is open with peer"));
+       // Even though the last funding_transaction_generated errored, it still generated a
+       // SendFundingCreated. However, when the peer responds with a funding_signed it will send the
+       // appropriate error message.
+       let as_funding_created = 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(), &as_funding_created);
+       check_added_monitors!(nodes[1], 1);
+       expect_channel_pending_event(&nodes[1], &nodes[0].node.get_our_node_id());
+       let reason = ClosureReason::ProcessingError { err: format!("An existing channel using outpoint {} is open with peer {}", funding_output, nodes[2].node.get_our_node_id()), };
+       check_closed_events(&nodes[0], &[ExpectedCloseEvent::from_id_reason(funding_output.to_channel_id(), true, reason)]);
+
+       let funding_signed = get_event_msg!(nodes[1], MessageSendEvent::SendFundingSigned, nodes[0].node.get_our_node_id());
+       nodes[0].node.handle_funding_signed(&nodes[1].node.get_our_node_id(), &funding_signed);
+       get_err_msg(&nodes[0], &nodes[1].node.get_our_node_id());
+}
+
 #[test]
 fn test_duplicate_funding_err_in_funding() {
        // Test that if we have a live channel with one peer, then another peer comes along and tries