Rewrite ChannelManager::handle_error

Exposing ChannelPhase in ChannelManager has led to verbose match
statements, which need to be modified each time a ChannelPhase is added.
Making ChannelPhase an implementation detail of Channel would help avoid
this.

As a step in this direction, update ChannelManager::handle_error to use
a new ChannelPhase::maybe_handle_error_without_close.

Author: jkczyz

lightning/src/ln/channel.rs

@@ -1279,6 +1279,38 @@ impl<'a, SP: Deref> ChannelPhase<SP> where
 			},
 		}
 	}
+
+	pub fn maybe_handle_error_without_close<F: Deref, L: Deref>(
+		&mut self, chain_hash: ChainHash, fee_estimator: &LowerBoundedFeeEstimator<F>, logger: &L,
+	) -> Result<Option<OpenChannelMessage>, ()>
+	where
+		F::Target: FeeEstimator,
+		L::Target: Logger,
+	{
+		match self {
+			ChannelPhase::Funded(_) => Ok(None),
+			ChannelPhase::UnfundedOutboundV1(chan) => {
+				let logger = WithChannelContext::from(logger, &chan.context, None);
+				chan.maybe_handle_error_without_close(chain_hash, fee_estimator, &&logger)
+					.map(|msg| Some(OpenChannelMessage::V1(msg)))
+			},
+			ChannelPhase::UnfundedInboundV1(_) => Ok(None),
+			#[cfg(dual_funding)]
+			ChannelPhase::UnfundedV2(chan) => {
+				if chan.context.is_outbound() {
+					chan.maybe_handle_error_without_close(chain_hash, fee_estimator)
+						.map(|msg| Some(OpenChannelMessage::V2(msg)))
+				} else {
+					Ok(None)
+				}
+			},
+			#[cfg(not(dual_funding))]
+			ChannelPhase::UnfundedV2(_) => {
+				debug_assert!(false);
+				Ok(None)
+			},
+		}
+	}
 }
 
 /// Contains all state common to unfunded inbound/outbound channels.
@@ -8979,6 +9011,7 @@ impl<SP: Deref> PendingV2Channel<SP> where SP::Target: SignerProvider {
 	/// If we receive an error message, it may only be a rejection of the channel type we tried,
 	/// not of our ability to open any channel at all. Thus, on error, we should first call this
 	/// and see if we get a new `OpenChannelV2` message, otherwise the channel is failed.
+	#[cfg(dual_funding)]
 	pub(crate) fn maybe_handle_error_without_close<F: Deref>(
 		&mut self, chain_hash: ChainHash, fee_estimator: &LowerBoundedFeeEstimator<F>
 	) -> Result<msgs::OpenChannelV2, ()>
@@ -8989,6 +9022,7 @@ impl<SP: Deref> PendingV2Channel<SP> where SP::Target: SignerProvider {
 		Ok(self.get_open_channel_v2(chain_hash))
 	}
 
+	#[cfg(dual_funding)]
 	pub fn get_open_channel_v2(&self, chain_hash: ChainHash) -> msgs::OpenChannelV2 {
 		if !self.context.is_outbound() {
 			debug_assert!(false, "Tried to send open_channel2 for an inbound channel?");

lightning/src/ln/channelmanager.rs

@@ -11760,28 +11760,27 @@ where
 				let mut peer_state_lock = peer_state_mutex_opt.unwrap().lock().unwrap();
 				let peer_state = &mut *peer_state_lock;
 				match peer_state.channel_by_id.get_mut(&msg.channel_id) {
-					Some(ChannelPhase::UnfundedOutboundV1(ref mut chan)) => {
-						let logger = WithChannelContext::from(&self.logger, &chan.context, None);
-						if let Ok(msg) = chan.maybe_handle_error_without_close(self.chain_hash, &self.fee_estimator, &&logger) {
+					Some(chan) => match chan.maybe_handle_error_without_close(
+						self.chain_hash, &self.fee_estimator, &self.logger,
+					) {
+						Ok(Some(OpenChannelMessage::V1(msg))) => {
 							peer_state.pending_msg_events.push(events::MessageSendEvent::SendOpenChannel {
 								node_id: counterparty_node_id,
 								msg,
 							});
 							return;
-						}
-					},
-					Some(ChannelPhase::UnfundedV2(ref mut chan)) => {
-						if chan.context.is_outbound() {
-							if let Ok(msg) = chan.maybe_handle_error_without_close(self.chain_hash, &self.fee_estimator) {
-								peer_state.pending_msg_events.push(events::MessageSendEvent::SendOpenChannelV2 {
-									node_id: counterparty_node_id,
-									msg,
-								});
-								return;
-							}
-						}
+						},
+						#[cfg(dual_funding)]
+						Ok(Some(OpenChannelMessage::V2(msg))) => {
+							peer_state.pending_msg_events.push(events::MessageSendEvent::SendOpenChannelV2 {
+								node_id: counterparty_node_id,
+								msg,
+							});
+							return;
+						},
+						Ok(None) | Err(()) => {},
 					},
-					None | Some(ChannelPhase::UnfundedInboundV1(_) | ChannelPhase::Funded(_)) => (),
+					None => {},
 				}
 			}