Do not panic if a peer learns our funding info before we fund

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.

Author: TheBlueMatt

lightning/src/ln/channelmanager.rs

@@ -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));
 			}

lightning/src/ln/functional_test_utils.rs

@@ -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)
 }
 

lightning/src/ln/functional_tests.rs

@@ -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