Rewrite ChannelManager::peer_connected

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::peer_connected to
use ChannelPhase::as_funded_mut and a new
ChannelPhase::maybe_get_open_channel method.

Author: jkczyz

lightning/src/ln/channel.rs

@@ -39,7 +39,7 @@ use crate::ln::msgs;
 use crate::ln::msgs::{ClosingSigned, ClosingSignedFeeRange, DecodeError};
 use crate::ln::script::{self, ShutdownScript};
 use crate::ln::channel_state::{ChannelShutdownState, CounterpartyForwardingInfo, InboundHTLCDetails, InboundHTLCStateDetails, OutboundHTLCDetails, OutboundHTLCStateDetails};
-use crate::ln::channelmanager::{self, PendingHTLCStatus, HTLCSource, SentHTLCId, HTLCFailureMsg, PendingHTLCInfo, RAACommitmentOrder, PaymentClaimDetails, BREAKDOWN_TIMEOUT, MIN_CLTV_EXPIRY_DELTA, MAX_LOCAL_BREAKDOWN_TIMEOUT};
+use crate::ln::channelmanager::{self, OpenChannelMessage, PendingHTLCStatus, HTLCSource, SentHTLCId, HTLCFailureMsg, PendingHTLCInfo, RAACommitmentOrder, PaymentClaimDetails, BREAKDOWN_TIMEOUT, MIN_CLTV_EXPIRY_DELTA, MAX_LOCAL_BREAKDOWN_TIMEOUT};
 use crate::ln::chan_utils::{
 	CounterpartyCommitmentSecrets, TxCreationKeys, HTLCOutputInCommitment, htlc_success_tx_weight,
 	htlc_timeout_tx_weight, make_funding_redeemscript, ChannelPublicKeys, CommitmentTransaction,
@@ -1242,6 +1242,43 @@ impl<'a, SP: Deref> ChannelPhase<SP> where
 			ChannelPhase::UnfundedV2(_) => false,
 		}
 	}
+
+	pub fn maybe_get_open_channel<L: Deref>(
+		&mut self, chain_hash: ChainHash, logger: &L,
+	) -> Option<OpenChannelMessage> where L::Target: Logger {
+		match self {
+			ChannelPhase::Funded(_) => None,
+			ChannelPhase::UnfundedOutboundV1(chan) => {
+				let logger = WithChannelContext::from(logger, &chan.context, None);
+				chan.get_open_channel(chain_hash, &&logger)
+					.map(|msg| OpenChannelMessage::V1(msg))
+			},
+			ChannelPhase::UnfundedInboundV1(_) => {
+				// Since unfunded inbound channel maps are cleared upon disconnecting a peer,
+				// they are not persisted and won't be recovered after a crash.
+				// Therefore, they shouldn't exist at this point.
+				debug_assert!(false);
+				None
+			},
+			#[cfg(dual_funding)]
+			ChannelPhase::UnfundedV2(chan) => {
+				if chan.context.is_outbound() {
+					Some(OpenChannelMessage::V2(chan.get_open_channel_v2(chain_hash)))
+				} else {
+					// Since unfunded inbound channel maps are cleared upon disconnecting a peer,
+					// they are not persisted and won't be recovered after a crash.
+					// Therefore, they shouldn't exist at this point.
+					debug_assert!(false);
+					None
+				}
+			},
+			#[cfg(not(dual_funding))]
+			ChannelPhase::UnfundedV2(_) => {
+				debug_assert!(false);
+				None
+			},
+		}
+	}
 }
 
 /// Contains all state common to unfunded inbound/outbound channels.

lightning/src/ln/channelmanager.rs

@@ -11646,41 +11646,29 @@ where
 				let pending_msg_events = &mut peer_state.pending_msg_events;
 
 				for (_, phase) in peer_state.channel_by_id.iter_mut() {
-					match phase {
-						ChannelPhase::Funded(chan) => {
+					match phase.as_funded_mut() {
+						Some(chan) => {
 							let logger = WithChannelContext::from(&self.logger, &chan.context, None);
 							pending_msg_events.push(events::MessageSendEvent::SendChannelReestablish {
 								node_id: chan.context.get_counterparty_node_id(),
 								msg: chan.get_channel_reestablish(&&logger),
 							});
 						},
-						ChannelPhase::UnfundedOutboundV1(chan) => {
-							let logger = WithChannelContext::from(&self.logger, &chan.context, None);
-							if let Some(msg) = chan.get_open_channel(self.chain_hash, &&logger) {
+						None => match phase.maybe_get_open_channel(self.chain_hash, &self.logger) {
+							Some(OpenChannelMessage::V1(msg)) => {
 								pending_msg_events.push(events::MessageSendEvent::SendOpenChannel {
-									node_id: chan.context.get_counterparty_node_id(),
+									node_id: phase.context().get_counterparty_node_id(),
 									msg,
 								});
-							}
-						},
-						ChannelPhase::UnfundedV2(chan) => {
-							if chan.context.is_outbound() {
+							},
+							#[cfg(dual_funding)]
+							Some(OpenChannelMessage::V2(msg)) => {
 								pending_msg_events.push(events::MessageSendEvent::SendOpenChannelV2 {
-									node_id: chan.context.get_counterparty_node_id(),
-									msg: chan.get_open_channel_v2(self.chain_hash),
+									node_id: phase.context().get_counterparty_node_id(),
+									msg,
 								});
-							} else {
-								// Since unfunded inbound channel maps are cleared upon disconnecting a peer,
-								// they are not persisted and won't be recovered after a crash.
-								// Therefore, they shouldn't exist at this point.
-								debug_assert!(false);
-							}
-						},
-						ChannelPhase::UnfundedInboundV1(_) => {
-							// Since unfunded inbound channel maps are cleared upon disconnecting a peer,
-							// they are not persisted and won't be recovered after a crash.
-							// Therefore, they shouldn't exist at this point.
-							debug_assert!(false);
+							},
+							None => {},
 						},
 					}
 				}