f Emit ChannelPending only after broadcast

Author: tnull

lightning/src/ln/chanmon_update_fail_tests.rs

@@ -1859,13 +1859,13 @@ fn do_during_funding_monitor_fail(confirm_a_first: bool, restore_b_before_conf:
 	chanmon_cfgs[0].persister.set_update_ret(ChannelMonitorUpdateStatus::InProgress);
 	nodes[0].node.handle_funding_signed(&nodes[1].node.get_our_node_id(), &get_event_msg!(nodes[1], MessageSendEvent::SendFundingSigned, nodes[0].node.get_our_node_id()));
 	check_added_monitors!(nodes[0], 1);
-	expect_channel_pending_event(&nodes[0], &nodes[1].node.get_our_node_id());
 	assert!(nodes[0].node.get_and_clear_pending_msg_events().is_empty());
 	assert!(nodes[0].node.get_and_clear_pending_events().is_empty());
 	chanmon_cfgs[0].persister.set_update_ret(ChannelMonitorUpdateStatus::Completed);
 	let (outpoint, latest_update, _) = nodes[0].chain_monitor.latest_monitor_update_id.lock().unwrap().get(&channel_id).unwrap().clone();
 	nodes[0].chain_monitor.chain_monitor.force_channel_monitor_updated(outpoint, latest_update);
 	check_added_monitors!(nodes[0], 0);
+	expect_channel_pending_event(&nodes[0], &nodes[1].node.get_our_node_id());
 
 	let events = nodes[0].node.get_and_clear_pending_events();
 	assert_eq!(events.len(), 0);
@@ -2820,7 +2820,6 @@ fn do_test_outbound_reload_without_init_mon(use_0conf: bool) {
 
 			nodes[0].node.handle_funding_signed(&nodes[1].node.get_our_node_id(), &msg);
 			check_added_monitors!(nodes[0], 1);
-			expect_channel_pending_event(&nodes[0], &nodes[1].node.get_our_node_id());
 		}
 		_ => panic!("Unexpected event"),
 	}

lightning/src/ln/channelmanager.rs

@@ -4253,8 +4253,6 @@ where
 			});
 		}
 
-		emit_channel_ready_event!(self, channel);
-
 		macro_rules! handle_cs { () => {
 			if let Some(update) = commitment_update {
 				pending_msg_events.push(events::MessageSendEvent::UpdateHTLCs {
@@ -4285,8 +4283,19 @@ where
 		if let Some(tx) = funding_broadcastable {
 			log_info!(self.logger, "Broadcasting funding transaction with txid {}", tx.txid());
 			self.tx_broadcaster.broadcast_transaction(&tx);
+
+			let mut pending_events = self.pending_events.lock().unwrap();
+			pending_events.push(events::Event::ChannelPending {
+				channel_id: channel.channel_id(),
+				former_temporary_channel_id: channel.temporary_channel_id(),
+				counterparty_node_id: counterparty_node_id,
+				user_channel_id: channel.get_user_id(),
+				funding_txo: channel.get_funding_txo().unwrap().into_bitcoin_outpoint(),
+			});
 		}
 
+		emit_channel_ready_event!(self, channel);
+
 		htlc_forwards
 	}
 
@@ -4673,10 +4682,7 @@ where
 			hash_map::Entry::Occupied(mut chan) => {
 				let monitor = try_chan_entry!(self,
 					chan.get_mut().funding_signed(&msg, best_block, &self.signer_provider, &self.logger), chan);
-				let funding_txo = chan.get().get_funding_txo().unwrap();
-				let user_channel_id = chan.get().get_user_id();
-				let former_temporary_channel_id = chan.get().temporary_channel_id();
-				let update_res = self.chain_monitor.watch_channel(funding_txo, monitor);
+				let update_res = self.chain_monitor.watch_channel(chan.get().get_funding_txo().unwrap(), monitor);
 				let mut res = handle_new_monitor_update!(self, update_res, 0, peer_state_lock, peer_state, per_peer_state, chan);
 				if let Err(MsgHandleErrInternal { ref mut shutdown_finish, .. }) = res {
 					// We weren't able to watch the channel to begin with, so no updates should be made on
@@ -4685,15 +4691,6 @@ where
 					if let Some((ref mut shutdown_finish, _)) = shutdown_finish {
 						shutdown_finish.0.take();
 					}
-				} else {
-					let mut pending_events = self.pending_events.lock().unwrap();
-					pending_events.push(events::Event::ChannelPending {
-						channel_id: msg.channel_id,
-						former_temporary_channel_id,
-						counterparty_node_id: *counterparty_node_id,
-						user_channel_id,
-						funding_txo: funding_txo.into_bitcoin_outpoint(),
-					});
 				}
 				res
 			},

lightning/src/ln/priv_short_conf_tests.rs

@@ -612,7 +612,6 @@ fn test_0conf_channel_with_async_monitor() {
 		MessageSendEvent::SendFundingSigned { node_id, msg } => {
 			assert_eq!(*node_id, nodes[0].node.get_our_node_id());
 			nodes[0].node.handle_funding_signed(&nodes[1].node.get_our_node_id(), &msg);
-			expect_channel_pending_event(&nodes[0], &nodes[1].node.get_our_node_id());
 			check_added_monitors!(nodes[0], 1);
 		}
 		_ => panic!("Unexpected event"),
@@ -629,6 +628,22 @@ fn test_0conf_channel_with_async_monitor() {
 
 	assert!(nodes[0].node.get_and_clear_pending_msg_events().is_empty());
 	nodes[0].chain_monitor.complete_sole_pending_chan_update(&channel_id);
+
+	let events = nodes[0].node.get_and_clear_pending_events();
+	assert_eq!(events.len(), 2);
+	match events[0] {
+		crate::events::Event::ChannelPending { ref counterparty_node_id, .. } => {
+			assert_eq!(nodes[1].node.get_our_node_id(), *counterparty_node_id);
+		},
+		_ => panic!("Unexpected event"),
+	}
+	match events[1] {
+		crate::events::Event::ChannelReady { ref counterparty_node_id, .. } => {
+			assert_eq!(nodes[1].node.get_our_node_id(), *counterparty_node_id);
+		},
+		_ => panic!("Unexpected event"),
+	}
+
 	let as_locked_update = nodes[0].node.get_and_clear_pending_msg_events();
 
 	// Note that the funding transaction is actually released when
@@ -643,7 +658,6 @@ fn test_0conf_channel_with_async_monitor() {
 		}
 		_ => panic!("Unexpected event"),
 	}
-	expect_channel_ready_event(&nodes[0], &nodes[1].node.get_our_node_id());
 	expect_channel_ready_event(&nodes[1], &nodes[0].node.get_our_node_id());
 
 	let bs_channel_update = get_event_msg!(nodes[1], MessageSendEvent::SendChannelUpdate, nodes[0].node.get_our_node_id());