From fdbce7f75b36dea66aaa95059737c4f29ecefb23 Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Tue, 7 Jan 2025 17:46:49 -0600 Subject: [PATCH] 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. --- lightning/src/ln/channel.rs | 39 +++++++++++++++++++++++++++++- lightning/src/ln/channelmanager.rs | 36 +++++++++------------------ 2 files changed, 50 insertions(+), 25 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index dfe05e769..8c40de801 100644 --- a/lightning/src/ln/channel.rs +++ b/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 where ChannelPhase::UnfundedV2(_) => false, } } + + pub fn maybe_get_open_channel( + &mut self, chain_hash: ChainHash, logger: &L, + ) -> Option 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. diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 9ad1ffd47..e5fc560a5 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/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 => {}, }, } }