From a8f83dc8637cc4c56298fdc7d6570867bf2a4d9f Mon Sep 17 00:00:00 2001 From: optout <13562139+optout21@users.noreply.github.com> Date: Thu, 21 Nov 2024 12:14:30 +0100 Subject: [PATCH] Handle transition to Funded inline, without remove+readd from map --- lightning/src/ln/channel.rs | 54 ++++++++++++++++++++++++------ lightning/src/ln/channelmanager.rs | 21 ++++++------ 2 files changed, 54 insertions(+), 21 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 661dbc22c76..4993b215928 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -1139,6 +1139,7 @@ impl<'a, SP: Deref> ChannelPhase where SP::Target: SignerProvider, ::EcdsaSigner: ChannelSigner, { + #[inline] pub fn context(&'a self) -> &'a ChannelContext { match self { ChannelPhase::Funded(chan) => &chan.context, @@ -1149,6 +1150,7 @@ impl<'a, SP: Deref> ChannelPhase where } } + #[inline] pub fn context_mut(&'a mut self) -> &'a mut ChannelContext { match self { ChannelPhase::Funded(ref mut chan) => &mut chan.context, @@ -1162,40 +1164,72 @@ impl<'a, SP: Deref> ChannelPhase where /// A top-level channel struct, containing a channel phase pub(super) struct ChannelWrapper where SP::Target: SignerProvider { - phase: ChannelPhase, + /// The inner channel phase + /// Option is used to facilitate in-place replacement (see e.g. move_v2_to_funded), + /// but it is never None, ensured in new() and set_phase() + phase: Option>, } impl<'a, SP: Deref> ChannelWrapper where SP::Target: SignerProvider { pub fn new(phase: ChannelPhase) -> Self { - Self { phase } + Self { + phase: Some(phase), + } } + #[inline] pub fn phase(&self) -> &ChannelPhase { - &self.phase + self.phase.as_ref().unwrap() } + #[inline] pub fn phase_mut(&mut self) -> &mut ChannelPhase { - &mut self.phase + self.phase.as_mut().unwrap() } pub fn phase_take(self) -> ChannelPhase { - self.phase + match self.phase { + Some(phase) => phase, + None => panic!("None phase"), + } } pub fn get_funded_channel(&self) -> Option<&Channel> { - if let ChannelPhase::Funded(chan) = &self.phase { + if let ChannelPhase::Funded(chan) = &self.phase.as_ref().unwrap() { Some(chan) } else { None } } + /// Change the internal phase + #[inline] + pub fn set_phase(&mut self, new_phase: ChannelPhase) { + self.phase = Some(new_phase); + } + + pub fn move_v2_to_funded(&mut self, signing_session: InteractiveTxSigningSession) -> Result<(), ChannelError> { + // We need a borrow to the phase field, but self is only a mut ref + let phase_inline = self.phase.take().unwrap(); + let new_phase = match phase_inline { + ChannelPhase::UnfundedOutboundV2(chan) => + ChannelPhase::Funded(chan.into_funded_channel(signing_session)?), + ChannelPhase::UnfundedInboundV2(chan) => + ChannelPhase::Funded(chan.into_funded_channel(signing_session)?), + _ => phase_inline, + }; + self.set_phase(new_phase); + Ok(()) + } + + #[inline] pub fn context(&'a self) -> &'a ChannelContext { - self.phase.context() + self.phase().context() } + #[inline] pub fn context_mut(&'a mut self) -> &'a mut ChannelContext { - self.phase.context_mut() + self.phase_mut().context_mut() } } @@ -8888,7 +8922,7 @@ impl OutboundV2Channel where SP::Target: SignerProvider { } } - pub fn into_channel(self, signing_session: InteractiveTxSigningSession) -> Result, ChannelError>{ + pub fn into_funded_channel(self, signing_session: InteractiveTxSigningSession) -> Result, ChannelError>{ let channel = Channel { context: self.context, interactive_tx_signing_session: Some(signing_session), @@ -9082,7 +9116,7 @@ impl InboundV2Channel where SP::Target: SignerProvider { self.generate_accept_channel_v2_message() } - pub fn into_channel(self, signing_session: InteractiveTxSigningSession) -> Result, ChannelError>{ + pub fn into_funded_channel(self, signing_session: InteractiveTxSigningSession) -> Result, ChannelError>{ let channel = Channel { context: self.context, interactive_tx_signing_session: Some(signing_session), diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index c2ca2d6a1bb..3eecd00d55e 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -5037,6 +5037,7 @@ where } } } else { + // put it back peer_state.channel_by_id.insert(temporary_channel_id, ChannelWrapper::new(phase)); return Err(APIError::APIMisuseError { err: format!( @@ -8368,18 +8369,16 @@ where "Got a tx_complete message with no interactive transaction construction expected or in-progress" .into())), }.map_err(|err| MsgHandleErrInternal::send_err_msg_no_close(format!("{}", err), msg.channel_id))?; - let (channel_id, channel) = chan_entry.remove_entry(); - let channel = match channel.phase_take() { - ChannelPhase::UnfundedOutboundV2(chan) => chan.into_channel(signing_session), - ChannelPhase::UnfundedInboundV2(chan) => chan.into_channel(signing_session), - _ => { + + // change the channel phase inline + match chan_entry.get_mut().move_v2_to_funded(signing_session) { + Ok(_) => {}, + Err(err) => { debug_assert!(false); // It cannot be another variant as we are in the `Ok` branch of the above match. - Err(ChannelError::Warn( - "Got a tx_complete message with no interactive transaction construction expected or in-progress" - .into())) - }, - }.map_err(|err| MsgHandleErrInternal::send_err_msg_no_close(format!("{}", err), msg.channel_id))?; - peer_state.channel_by_id.insert(channel_id, ChannelWrapper::new(ChannelPhase::Funded(channel))); + return Err(MsgHandleErrInternal::send_err_msg_no_close(format!("Got a tx_complete message, could not transition to funding, {}", err), msg.channel_id))?; + } + } + if let Some(funding_ready_for_sig_event) = funding_ready_for_sig_event_opt { let mut pending_events = self.pending_events.lock().unwrap(); pending_events.push_back((funding_ready_for_sig_event, None));