f swap check order

Author: TheBlueMatt

lightning/src/ln/channelmanager.rs

@@ -5708,53 +5708,55 @@ where
 				Err(MsgHandleErrInternal::send_err_msg_no_close("Already had channel with the new channel_id".to_owned(), funding_msg.channel_id))
 			},
 			hash_map::Entry::Vacant(e) => {
-				let monitor_res = self.chain_monitor.watch_channel(monitor.get_funding_txo().0, monitor);
-				if let Ok(persist_state) = monitor_res {
-					match self.id_to_peer.lock().unwrap().entry(chan.context.channel_id()) {
-						hash_map::Entry::Occupied(_) => {
-							return Err(MsgHandleErrInternal::send_err_msg_no_close(
-								"The funding_created message had the same funding_txid as an existing channel - funding is not possible".to_owned(),
-								funding_msg.channel_id))
-						},
-						hash_map::Entry::Vacant(i_e) => {
+				let mut id_to_peer_lock = self.id_to_peer.lock().unwrap();
+				match id_to_peer_lock.entry(chan.context.channel_id()) {
+					hash_map::Entry::Occupied(_) => {
+						return Err(MsgHandleErrInternal::send_err_msg_no_close(
+							"The funding_created message had the same funding_txid as an existing channel - funding is not possible".to_owned(),
+							funding_msg.channel_id))
+					},
+					hash_map::Entry::Vacant(i_e) => {
+						let monitor_res = self.chain_monitor.watch_channel(monitor.get_funding_txo().0, monitor);
+						if let Ok(persist_state) = monitor_res {
 							i_e.insert(chan.context.get_counterparty_node_id());
-						}
-					}
-
-					// There's no problem signing a counterparty's funding transaction if our monitor
-					// hasn't persisted to disk yet - we can't lose money on a transaction that we haven't
-					// accepted payment from yet. We do, however, need to wait to send our channel_ready
-					// until we have persisted our monitor.
-					let new_channel_id = funding_msg.channel_id;
-					peer_state.pending_msg_events.push(events::MessageSendEvent::SendFundingSigned {
-						node_id: counterparty_node_id.clone(),
-						msg: funding_msg,
-					});
+							mem::drop(id_to_peer_lock);
+
+							// There's no problem signing a counterparty's funding transaction if our monitor
+							// hasn't persisted to disk yet - we can't lose money on a transaction that we haven't
+							// accepted payment from yet. We do, however, need to wait to send our channel_ready
+							// until we have persisted our monitor.
+							let new_channel_id = funding_msg.channel_id;
+							peer_state.pending_msg_events.push(events::MessageSendEvent::SendFundingSigned {
+								node_id: counterparty_node_id.clone(),
+								msg: funding_msg,
+							});
 
-					if let ChannelPhase::Funded(chan) = e.insert(ChannelPhase::Funded(chan)) {
-						let mut res = handle_new_monitor_update!(self, persist_state, peer_state_lock, peer_state,
-							per_peer_state, chan, MANUALLY_REMOVING_INITIAL_MONITOR,
-							{ peer_state.channel_by_id.remove(&new_channel_id) });
-
-						// Note that we reply with the new channel_id in error messages if we gave up on the
-						// channel, not the temporary_channel_id. This is compatible with ourselves, but the
-						// spec is somewhat ambiguous here. Not a huge deal since we'll send error messages for
-						// any messages referencing a previously-closed channel anyway.
-						// We do not propagate the monitor update to the user as it would be for a monitor
-						// that we didn't manage to store (and that we don't care about - we don't respond
-						// with the funding_signed so the channel can never go on chain).
-						if let Err(MsgHandleErrInternal { shutdown_finish: Some((res, _)), .. }) = &mut res {
-							res.0 = None;
+							if let ChannelPhase::Funded(chan) = e.insert(ChannelPhase::Funded(chan)) {
+								let mut res = handle_new_monitor_update!(self, persist_state, peer_state_lock, peer_state,
+									per_peer_state, chan, MANUALLY_REMOVING_INITIAL_MONITOR,
+									{ peer_state.channel_by_id.remove(&new_channel_id) });
+
+								// Note that we reply with the new channel_id in error messages if we gave up on the
+								// channel, not the temporary_channel_id. This is compatible with ourselves, but the
+								// spec is somewhat ambiguous here. Not a huge deal since we'll send error messages for
+								// any messages referencing a previously-closed channel anyway.
+								// We do not propagate the monitor update to the user as it would be for a monitor
+								// that we didn't manage to store (and that we don't care about - we don't respond
+								// with the funding_signed so the channel can never go on chain).
+								if let Err(MsgHandleErrInternal { shutdown_finish: Some((res, _)), .. }) = &mut res {
+									res.0 = None;
+								}
+								res.map(|_| ())
+							} else {
+								unreachable!("This must be a funded channel as we just inserted it.");
+							}
+						} else {
+							log_error!(self.logger, "Persisting initial ChannelMonitor failed, implying the funding outpoint was duplicated");
+							return Err(MsgHandleErrInternal::send_err_msg_no_close(
+								"The funding_created message had the same funding_txid as an existing channel - funding is not possible".to_owned(),
+								funding_msg.channel_id));
 						}
-						res.map(|_| ())
-					} else {
-						unreachable!("This must be a funded channel as we just inserted it.");
 					}
-				} else {
-					log_error!(self.logger, "Persisting initial ChannelMonitor failed, implying the funding outpoint was duplicated");
-					return Err(MsgHandleErrInternal::send_err_msg_no_close(
-						"The funding_created message had the same funding_txid as an existing channel - funding is not possible".to_owned(),
-						funding_msg.channel_id));
 				}
 			}
 		}