diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index ca22c975503..36c41437eb9 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -8642,10 +8642,7 @@ where pub fn maybe_propose_closing_signed( &mut self, fee_estimator: &LowerBoundedFeeEstimator, logger: &L, - ) -> Result< - (Option, Option, Option), - ChannelError, - > + ) -> Result<(Option, Option<(Transaction, ShutdownResult)>), ChannelError> where F::Target: FeeEstimator, L::Target: Logger, @@ -8655,20 +8652,20 @@ where // initiate `closing_signed` negotiation until we're clear of all pending messages. Note // that closing_negotiation_ready checks this case (as well as a few others). if self.context.last_sent_closing_fee.is_some() || !self.closing_negotiation_ready() { - return Ok((None, None, None)); + return Ok((None, None)); } if !self.funding.is_outbound() { if let Some(msg) = &self.context.pending_counterparty_closing_signed.take() { return self.closing_signed(fee_estimator, &msg, logger); } - return Ok((None, None, None)); + return Ok((None, None)); } // If we're waiting on a counterparty `commitment_signed` to clear some updates from our // local commitment transaction, we can't yet initiate `closing_signed` negotiation. if self.context.expecting_peer_commitment_signed { - return Ok((None, None, None)); + return Ok((None, None)); } let (our_min_fee, our_max_fee) = self.calculate_closing_fee_limits(fee_estimator); @@ -8687,7 +8684,7 @@ where our_max_fee, logger, ); - Ok((closing_signed, None, None)) + Ok((closing_signed, None)) } fn mark_response_received(&mut self) { @@ -8950,10 +8947,7 @@ where pub fn closing_signed( &mut self, fee_estimator: &LowerBoundedFeeEstimator, msg: &msgs::ClosingSigned, logger: &L, - ) -> Result< - (Option, Option, Option), - ChannelError, - > + ) -> Result<(Option, Option<(Transaction, ShutdownResult)>), ChannelError> where F::Target: FeeEstimator, L::Target: Logger, @@ -8993,7 +8987,7 @@ where if self.context.channel_state.is_monitor_update_in_progress() { self.context.pending_counterparty_closing_signed = Some(msg.clone()); - return Ok((None, None, None)); + return Ok((None, None)); } let funding_redeemscript = self.funding.get_funding_redeemscript(); @@ -9047,7 +9041,7 @@ where self.build_signed_closing_transaction(&mut closing_tx, &msg.signature, &sig); self.context.channel_state = ChannelState::ShutdownComplete; self.context.update_time_counter += 1; - return Ok((None, Some(tx), Some(shutdown_result))); + return Ok((None, Some((tx, shutdown_result)))); } } @@ -9070,26 +9064,25 @@ where our_max_fee, logger, ); - let (signed_tx, shutdown_result) = if $new_fee == msg.fee_satoshis { - let shutdown_result = - closing_signed.as_ref().map(|_| self.shutdown_result_coop_close()); - if closing_signed.is_some() { - self.context.channel_state = ChannelState::ShutdownComplete; - } + let signed_tx_shutdown = if $new_fee == msg.fee_satoshis { self.context.update_time_counter += 1; self.context.last_received_closing_sig = Some(msg.signature.clone()); - let tx = closing_signed.as_ref().map(|ClosingSigned { signature, .. }| { - self.build_signed_closing_transaction( + if let Some(ClosingSigned { signature, .. }) = &closing_signed { + let shutdown_result = self.shutdown_result_coop_close(); + self.context.channel_state = ChannelState::ShutdownComplete; + let tx = self.build_signed_closing_transaction( &closing_tx, &msg.signature, signature, - ) - }); - (tx, shutdown_result) + ); + Some((tx, shutdown_result)) + } else { + None + } } else { - (None, None) + None }; - return Ok((closing_signed, signed_tx, shutdown_result)) + return Ok((closing_signed, signed_tx_shutdown)) }; } diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index d0d429a0abc..e3dccb467ef 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -3217,16 +3217,16 @@ macro_rules! locked_close_channel { /// Returns (boolean indicating if we should remove the Channel object from memory, a mapped error) #[rustfmt::skip] macro_rules! convert_channel_err { - ($self: ident, $peer_state: expr, $err: expr, $chan: expr, $close: expr, $locked_close: expr, $channel_id: expr, _internal) => { + ($self: ident, $peer_state: expr, $err: expr, $chan: expr, $close: expr, $locked_close: expr, $channel_id: expr, _internal) => { { match $err { ChannelError::Warn(msg) => { - (false, MsgHandleErrInternal::from_chan_no_close(ChannelError::Warn(msg), *$channel_id)) + (false, MsgHandleErrInternal::from_chan_no_close(ChannelError::Warn(msg), $channel_id)) }, ChannelError::WarnAndDisconnect(msg) => { - (false, MsgHandleErrInternal::from_chan_no_close(ChannelError::WarnAndDisconnect(msg), *$channel_id)) + (false, MsgHandleErrInternal::from_chan_no_close(ChannelError::WarnAndDisconnect(msg), $channel_id)) }, ChannelError::Ignore(msg) => { - (false, MsgHandleErrInternal::from_chan_no_close(ChannelError::Ignore(msg), *$channel_id)) + (false, MsgHandleErrInternal::from_chan_no_close(ChannelError::Ignore(msg), $channel_id)) }, ChannelError::Close((msg, reason)) => { let (mut shutdown_res, chan_update) = $close(reason); @@ -3234,15 +3234,34 @@ macro_rules! convert_channel_err { log_error!(logger, "Closed channel {} due to close-required error: {}", $channel_id, msg); $locked_close(&mut shutdown_res, $chan); let err = - MsgHandleErrInternal::from_finish_shutdown(msg, *$channel_id, shutdown_res, chan_update); + MsgHandleErrInternal::from_finish_shutdown(msg, $channel_id, shutdown_res, chan_update); (true, err) }, ChannelError::SendError(msg) => { - (false, MsgHandleErrInternal::from_chan_no_close(ChannelError::SendError(msg), *$channel_id)) + (false, MsgHandleErrInternal::from_chan_no_close(ChannelError::SendError(msg), $channel_id)) }, } - }; - ($self: ident, $peer_state: expr, $err: expr, $funded_channel: expr, $channel_id: expr, FUNDED_CHANNEL) => { { + } }; + ($self: ident, $peer_state: expr, $shutdown_result: expr, $funded_channel: expr, COOP_CLOSED) => { { + let chan_id = $funded_channel.context.channel_id(); + let reason = ChannelError::Close(("Coop Closed".to_owned(), $shutdown_result.closure_reason.clone())); + let do_close = |_| { + ( + $shutdown_result, + $self.get_channel_update_for_broadcast(&$funded_channel).ok(), + ) + }; + let mut locked_close = |shutdown_res_mut: &mut ShutdownResult, funded_channel: &mut FundedChannel<_>| { + locked_close_channel!($self, $peer_state, funded_channel, shutdown_res_mut, FUNDED); + }; + let (close, mut err) = + convert_channel_err!($self, $peer_state, reason, $funded_channel, do_close, locked_close, chan_id, _internal); + err.dont_send_error_message(); + debug_assert!(close); + (close, err) + } }; + ($self: ident, $peer_state: expr, $err: expr, $funded_channel: expr, FUNDED_CHANNEL) => { { + let chan_id = $funded_channel.context.channel_id(); let mut do_close = |reason| { ( $funded_channel.force_shutdown(reason), @@ -3252,20 +3271,21 @@ macro_rules! convert_channel_err { let mut locked_close = |shutdown_res_mut: &mut ShutdownResult, funded_channel: &mut FundedChannel<_>| { locked_close_channel!($self, $peer_state, funded_channel, shutdown_res_mut, FUNDED); }; - convert_channel_err!($self, $peer_state, $err, $funded_channel, do_close, locked_close, $channel_id, _internal) + convert_channel_err!($self, $peer_state, $err, $funded_channel, do_close, locked_close, chan_id, _internal) } }; - ($self: ident, $peer_state: expr, $err: expr, $channel: expr, $channel_id: expr, UNFUNDED_CHANNEL) => { { + ($self: ident, $peer_state: expr, $err: expr, $channel: expr, UNFUNDED_CHANNEL) => { { + let chan_id = $channel.context().channel_id(); let mut do_close = |reason| { ($channel.force_shutdown(reason), None) }; let locked_close = |_, chan: &mut Channel<_>| { locked_close_channel!($self, chan.context(), UNFUNDED); }; - convert_channel_err!($self, $peer_state, $err, $channel, do_close, locked_close, $channel_id, _internal) + convert_channel_err!($self, $peer_state, $err, $channel, do_close, locked_close, chan_id, _internal) } }; - ($self: ident, $peer_state: expr, $err: expr, $channel: expr, $channel_id: expr) => { + ($self: ident, $peer_state: expr, $err: expr, $channel: expr) => { match $channel.as_funded_mut() { Some(funded_channel) => { - convert_channel_err!($self, $peer_state, $err, funded_channel, $channel_id, FUNDED_CHANNEL) + convert_channel_err!($self, $peer_state, $err, funded_channel, FUNDED_CHANNEL) }, None => { - convert_channel_err!($self, $peer_state, $err, $channel, $channel_id, UNFUNDED_CHANNEL) + convert_channel_err!($self, $peer_state, $err, $channel, UNFUNDED_CHANNEL) }, } }; @@ -3276,9 +3296,7 @@ macro_rules! break_channel_entry { match $res { Ok(res) => res, Err(e) => { - let key = *$entry.key(); - let (drop, res) = - convert_channel_err!($self, $peer_state, e, $entry.get_mut(), &key); + let (drop, res) = convert_channel_err!($self, $peer_state, e, $entry.get_mut()); if drop { $entry.remove_entry(); } @@ -3293,9 +3311,7 @@ macro_rules! try_channel_entry { match $res { Ok(res) => res, Err(e) => { - let key = *$entry.key(); - let (drop, res) = - convert_channel_err!($self, $peer_state, e, $entry.get_mut(), &key); + let (drop, res) = convert_channel_err!($self, $peer_state, e, $entry.get_mut()); if drop { $entry.remove_entry(); } @@ -4137,7 +4153,7 @@ where let reason = ClosureReason::LocallyCoopClosedUnfundedChannel; let err = ChannelError::Close((reason.to_string(), reason)); let mut chan = chan_entry.remove(); - let (_, mut e) = convert_channel_err!(self, peer_state, err, &mut chan, chan_id); + let (_, mut e) = convert_channel_err!(self, peer_state, err, &mut chan); e.dont_send_error_message(); shutdown_result = Err(e); } @@ -4334,7 +4350,7 @@ where if let Some(mut chan) = peer_state.channel_by_id.remove(&channel_id) { let reason = ClosureReason::FundingBatchClosure; let err = ChannelError::Close((reason.to_string(), reason)); - let (_, e) = convert_channel_err!(self, peer_state, err, &mut chan, &channel_id); + let (_, e) = convert_channel_err!(self, peer_state, err, &mut chan); shutdown_results.push((Err(e), counterparty_node_id)); } } @@ -4398,7 +4414,7 @@ where if let Some(mut chan) = peer_state.channel_by_id.remove(channel_id) { log_error!(logger, "Force-closing channel {}", channel_id); let err = ChannelError::Close((message, reason)); - let (_, mut e) = convert_channel_err!(self, peer_state, err, &mut chan, channel_id); + let (_, mut e) = convert_channel_err!(self, peer_state, err, &mut chan); mem::drop(peer_state_lock); mem::drop(per_peer_state); if is_from_counterparty { @@ -5877,7 +5893,7 @@ where let reason = ClosureReason::ProcessingError { err: e.clone() }; let err = ChannelError::Close((e.clone(), reason)); let (_, e) = - convert_channel_err!(self, peer_state, err, &mut chan, &channel_id); + convert_channel_err!(self, peer_state, err, &mut chan); shutdown_results.push((Err(e), counterparty_node_id)); }); } @@ -7045,7 +7061,7 @@ where if chan_needs_persist == NotifyOption::DoPersist { should_persist = NotifyOption::DoPersist; } if let Err(e) = funded_chan.timer_check_closing_negotiation_progress() { - let (needs_close, err) = convert_channel_err!(self, peer_state, e, funded_chan, chan_id, FUNDED_CHANNEL); + let (needs_close, err) = convert_channel_err!(self, peer_state, e, funded_chan, FUNDED_CHANNEL); handle_errors.push((Err(err), counterparty_node_id)); if needs_close { return false; } } @@ -7123,7 +7139,7 @@ where let reason = ClosureReason::FundingTimedOut; let msg = "Force-closing pending channel due to timeout awaiting establishment handshake".to_owned(); let err = ChannelError::Close((msg, reason)); - let (_, e) = convert_channel_err!(self, peer_state, err, chan, chan_id); + let (_, e) = convert_channel_err!(self, peer_state, err, chan); handle_errors.push((Err(e), counterparty_node_id)); false } else { @@ -8709,18 +8725,15 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ // above so at this point we just need to clean up any lingering entries // concerning this channel as it is safe to do so. debug_assert!(matches!(err, ChannelError::Close(_))); - // Really we should be returning the channel_id the peer expects based - // on their funding info here, but they're horribly confused anyway, so - // there's not a lot we can do to save them. let mut chan = Channel::from(inbound_chan); - return Err(convert_channel_err!(self, peer_state, err, &mut chan, &msg.temporary_channel_id, UNFUNDED_CHANNEL).1); + return Err(convert_channel_err!(self, peer_state, err, &mut chan).1); }, } }, Some(Err(mut chan)) => { let err_msg = format!("Got an unexpected funding_created message from peer with counterparty_node_id {}", counterparty_node_id); let err = ChannelError::close(err_msg); - return Err(convert_channel_err!(self, peer_state, err, &mut chan, &msg.temporary_channel_id).1); + return Err(convert_channel_err!(self, peer_state, err, &mut chan).1); }, None => return Err(MsgHandleErrInternal::send_err_msg_no_close(format!("Got a message for a channel from the wrong node! No such channel for the passed counterparty_node_id {}", counterparty_node_id), msg.temporary_channel_id)) }; @@ -8736,7 +8749,7 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ let err = ChannelError::close($err.to_owned()); chan.unset_funding_info(); let mut chan = Channel::from(chan); - return Err(convert_channel_err!(self, peer_state, err, &mut chan, &funded_channel_id, UNFUNDED_CHANNEL).1); + return Err(convert_channel_err!(self, peer_state, err, &mut chan, UNFUNDED_CHANNEL).1); } } } match peer_state.channel_by_id.entry(funded_channel_id) { @@ -9276,8 +9289,7 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ let reason = ClosureReason::CounterpartyCoopClosedUnfundedChannel; let err = ChannelError::Close((reason.to_string(), reason)); let mut chan = chan_entry.remove(); - let (_, mut e) = - convert_channel_err!(self, peer_state, err, &mut chan, &msg.channel_id); + let (_, mut e) = convert_channel_err!(self, peer_state, err, &mut chan); e.dont_send_error_message(); return Err(e); }, @@ -9312,33 +9324,35 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ msg.channel_id, ) })?; - let (tx, chan_option, shutdown_result) = { + let logger; + let tx_err: Option<(_, Result)> = { let mut peer_state_lock = peer_state_mutex.lock().unwrap(); let peer_state = &mut *peer_state_lock; match peer_state.channel_by_id.entry(msg.channel_id.clone()) { hash_map::Entry::Occupied(mut chan_entry) => { if let Some(chan) = chan_entry.get_mut().as_funded_mut() { - let logger = WithChannelContext::from(&self.logger, &chan.context, None); - let (closing_signed, tx, shutdown_result) = try_channel_entry!(self, peer_state, chan.closing_signed(&self.fee_estimator, &msg, &&logger), chan_entry); - debug_assert_eq!(shutdown_result.is_some(), chan.is_shutdown()); + logger = WithChannelContext::from(&self.logger, &chan.context, None); + let res = chan.closing_signed(&self.fee_estimator, &msg, &&logger); + let (closing_signed, tx_shutdown_result) = + try_channel_entry!(self, peer_state, res, chan_entry); + debug_assert_eq!(tx_shutdown_result.is_some(), chan.is_shutdown()); if let Some(msg) = closing_signed { peer_state.pending_msg_events.push(MessageSendEvent::SendClosingSigned { node_id: counterparty_node_id.clone(), msg, }); } - if let Some(mut close_res) = shutdown_result { + if let Some((tx, close_res)) = tx_shutdown_result { // We're done with this channel, we've got a signed closing transaction and // will send the closing_signed back to the remote peer upon return. This // also implies there are no pending HTLCs left on the channel, so we can // fully delete it from tracking (the channel monitor is still around to // watch for old state broadcasts)! - debug_assert!(tx.is_some()); - locked_close_channel!(self, peer_state, chan, close_res, FUNDED); - (tx, Some(chan_entry.remove()), Some(close_res)) + let (_, err) = convert_channel_err!(self, peer_state, close_res, chan, COOP_CLOSED); + chan_entry.remove(); + Some((tx, Err(err))) } else { - debug_assert!(tx.is_none()); - (tx, None, None) + None } } else { return try_channel_entry!(self, peer_state, Err(ChannelError::close( @@ -9348,26 +9362,11 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ hash_map::Entry::Vacant(_) => return Err(MsgHandleErrInternal::send_err_msg_no_close(format!("Got a message for a channel from the wrong node! No such channel for the passed counterparty_node_id {}", counterparty_node_id), msg.channel_id)) } }; - if let Some(broadcast_tx) = tx { - let channel_id = chan_option.as_ref().map(|channel| channel.context().channel_id()); - log_info!( - WithContext::from(&self.logger, Some(*counterparty_node_id), channel_id, None), - "Broadcasting {}", - log_tx!(broadcast_tx) - ); - self.tx_broadcaster.broadcast_transactions(&[&broadcast_tx]); - } - if let Some(chan) = chan_option.as_ref().and_then(Channel::as_funded) { - if let Ok(update) = self.get_channel_update_for_broadcast(chan) { - let mut pending_broadcast_messages = - self.pending_broadcast_messages.lock().unwrap(); - pending_broadcast_messages - .push(MessageSendEvent::BroadcastChannelUpdate { msg: update }); - } - } mem::drop(per_peer_state); - if let Some(shutdown_result) = shutdown_result { - self.finish_close_channel(shutdown_result); + if let Some((broadcast_tx, err)) = tx_err { + log_info!(logger, "Broadcasting {}", log_tx!(broadcast_tx)); + self.tx_broadcaster.broadcast_transactions(&[&broadcast_tx]); + let _ = handle_error!(self, err, *counterparty_node_id); } Ok(()) } @@ -10308,7 +10307,7 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ }; let err = ChannelError::Close((reason.to_string(), reason)); let mut chan = chan_entry.remove(); - let (_, e) = convert_channel_err!(self, peer_state, err, &mut chan, &channel_id); + let (_, e) = convert_channel_err!(self, peer_state, err, &mut chan); failed_channels.push((Err(e), counterparty_node_id)); } } @@ -10462,12 +10461,6 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ if let Some(broadcast_tx) = msgs.signed_closing_tx { log_info!(logger, "Broadcasting closing tx {}", log_tx!(broadcast_tx)); self.tx_broadcaster.broadcast_transactions(&[&broadcast_tx]); - - if let Ok(update) = self.get_channel_update_for_broadcast(&funded_chan) { - pending_msg_events.push(MessageSendEvent::BroadcastChannelUpdate { - msg: update - }); - } } } else { // We don't know how to handle a channel_ready or signed_closing_tx for a @@ -10481,14 +10474,14 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ } }; - let mut shutdown_results = Vec::new(); + let mut shutdown_results: Vec<(Result, _)> = Vec::new(); let per_peer_state = self.per_peer_state.read().unwrap(); let per_peer_state_iter = per_peer_state.iter().filter(|(cp_id, _)| { if let Some((counterparty_node_id, _)) = channel_opt { **cp_id == counterparty_node_id } else { true } }); - for (_cp_id, peer_state_mutex) in per_peer_state_iter { + for (cp_id, peer_state_mutex) in per_peer_state_iter { let mut peer_state_lock = peer_state_mutex.lock().unwrap(); let peer_state = &mut *peer_state_lock; peer_state.channel_by_id.retain(|_, chan| { @@ -10496,16 +10489,21 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ Some((_, channel_id)) if chan.context().channel_id() != channel_id => None, _ => unblock_chan(chan, &mut peer_state.pending_msg_events), }; - if let Some(mut shutdown_result) = shutdown_result { + if let Some(shutdown_res) = shutdown_result { let context = chan.context(); let logger = WithChannelContext::from(&self.logger, context, None); - log_trace!(logger, "Removing channel {} now that the signer is unblocked", context.channel_id()); - if let Some(funded_channel) = chan.as_funded_mut() { - locked_close_channel!(self, peer_state, funded_channel, shutdown_result, FUNDED); + let chan_id = context.channel_id(); + log_trace!(logger, "Removing channel {} now that the signer is unblocked", chan_id); + let (remove, err) = if let Some(funded_channel) = chan.as_funded_mut() { + convert_channel_err!(self, peer_state, shutdown_res, funded_channel, COOP_CLOSED) } else { - locked_close_channel!(self, chan.context(), UNFUNDED); - } - shutdown_results.push(shutdown_result); + debug_assert!(false); + let reason = shutdown_res.closure_reason.clone(); + let err = ChannelError::Close((reason.to_string(), reason)); + convert_channel_err!(self, peer_state, err, chan, UNFUNDED_CHANNEL) + }; + debug_assert!(remove); + shutdown_results.push((Err(err), *cp_id)); false } else { true @@ -10513,8 +10511,8 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ }); } drop(per_peer_state); - for shutdown_result in shutdown_results.drain(..) { - self.finish_close_channel(shutdown_result); + for (err, counterparty_node_id) in shutdown_results { + let _ = handle_error!(self, err, counterparty_node_id); } } @@ -10525,40 +10523,31 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ fn maybe_generate_initial_closing_signed(&self) -> bool { let mut handle_errors: Vec<(PublicKey, Result<(), _>)> = Vec::new(); let mut has_update = false; - let mut shutdown_results = Vec::new(); { let per_peer_state = self.per_peer_state.read().unwrap(); - for (_cp_id, peer_state_mutex) in per_peer_state.iter() { + for (cp_id, peer_state_mutex) in per_peer_state.iter() { let mut peer_state_lock = peer_state_mutex.lock().unwrap(); let peer_state = &mut *peer_state_lock; let pending_msg_events = &mut peer_state.pending_msg_events; - peer_state.channel_by_id.retain(|channel_id, chan| { + peer_state.channel_by_id.retain(|_, chan| { match chan.as_funded_mut() { Some(funded_chan) => { let logger = WithChannelContext::from(&self.logger, &funded_chan.context, None); match funded_chan.maybe_propose_closing_signed(&self.fee_estimator, &&logger) { - Ok((msg_opt, tx_opt, shutdown_result_opt)) => { + Ok((msg_opt, tx_shutdown_result_opt)) => { if let Some(msg) = msg_opt { has_update = true; pending_msg_events.push(MessageSendEvent::SendClosingSigned { node_id: funded_chan.context.get_counterparty_node_id(), msg, }); } - debug_assert_eq!(shutdown_result_opt.is_some(), funded_chan.is_shutdown()); - if let Some(mut shutdown_result) = shutdown_result_opt { - locked_close_channel!(self, peer_state, funded_chan, shutdown_result, FUNDED); - shutdown_results.push(shutdown_result); - } - if let Some(tx) = tx_opt { + debug_assert_eq!(tx_shutdown_result_opt.is_some(), funded_chan.is_shutdown()); + if let Some((tx, shutdown_res)) = tx_shutdown_result_opt { // We're done with this channel. We got a closing_signed and sent back // a closing_signed with a closing transaction to broadcast. - if let Ok(update) = self.get_channel_update_for_broadcast(&funded_chan) { - let mut pending_broadcast_messages = self.pending_broadcast_messages.lock().unwrap(); - pending_broadcast_messages.push(MessageSendEvent::BroadcastChannelUpdate { - msg: update - }); - } + let (_, err) = convert_channel_err!(self, peer_state, shutdown_res, funded_chan, COOP_CLOSED); + handle_errors.push((*cp_id, Err(err))); log_info!(logger, "Broadcasting {}", log_tx!(tx)); self.tx_broadcaster.broadcast_transactions(&[&tx]); @@ -10567,7 +10556,7 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ }, Err(e) => { has_update = true; - let (close_channel, res) = convert_channel_err!(self, peer_state, e, funded_chan, channel_id, FUNDED_CHANNEL); + let (close_channel, res) = convert_channel_err!(self, peer_state, e, funded_chan, FUNDED_CHANNEL); handle_errors.push((funded_chan.context.get_counterparty_node_id(), Err(res))); !close_channel } @@ -10579,14 +10568,10 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ } } - for (counterparty_node_id, err) in handle_errors.drain(..) { + for (counterparty_node_id, err) in handle_errors { let _ = handle_error!(self, err, counterparty_node_id); } - for shutdown_result in shutdown_results.drain(..) { - self.finish_close_channel(shutdown_result); - } - has_update } @@ -11825,7 +11810,7 @@ where let mut peer_state_lock = peer_state_mutex.lock().unwrap(); let peer_state = &mut *peer_state_lock; let pending_msg_events = &mut peer_state.pending_msg_events; - peer_state.channel_by_id.retain(|chan_id, chan| { + peer_state.channel_by_id.retain(|_, chan| { let logger = WithChannelContext::from(&self.logger, &chan.context(), None); if chan.peer_disconnected_is_resumable(&&logger) { return true; @@ -11833,7 +11818,7 @@ where // Clean up for removal. let reason = ClosureReason::DisconnectedPeer; let err = ChannelError::Close((reason.to_string(), reason)); - let (_, e) = convert_channel_err!(self, peer_state, err, chan, chan_id); + let (_, e) = convert_channel_err!(self, peer_state, err, chan); failed_channels.push((Err(e), counterparty_node_id)); false }); @@ -12386,7 +12371,7 @@ where let peer_state = &mut *peer_state_lock; let pending_msg_events = &mut peer_state.pending_msg_events; - peer_state.channel_by_id.retain(|chan_id, chan| { + peer_state.channel_by_id.retain(|channel_id, chan| { match chan.as_funded_mut() { // Retain unfunded channels. None => true, @@ -12397,14 +12382,14 @@ where let reason = LocalHTLCFailureReason::CLTVExpiryTooSoon; let data = self.get_htlc_inbound_temp_fail_data(reason); timed_out_htlcs.push((source, payment_hash, HTLCFailReason::reason(reason, data), - HTLCHandlingFailureType::Forward { node_id: Some(funded_channel.context.get_counterparty_node_id()), channel_id: funded_channel.context.channel_id() })); + HTLCHandlingFailureType::Forward { node_id: Some(funded_channel.context.get_counterparty_node_id()), channel_id: *channel_id })); } let logger = WithChannelContext::from(&self.logger, &funded_channel.context, None); match funding_confirmed_opt { Some(FundingConfirmedMessage::Establishment(channel_ready)) => { send_channel_ready!(self, pending_msg_events, funded_channel, channel_ready); if funded_channel.context.is_usable() { - log_trace!(logger, "Sending channel_ready with private initial channel_update for our counterparty on channel {}", funded_channel.context.channel_id()); + log_trace!(logger, "Sending channel_ready with private initial channel_update for our counterparty on channel {}", channel_id); if let Ok(msg) = self.get_channel_update_for_unicast(funded_channel) { pending_msg_events.push(MessageSendEvent::SendChannelUpdate { node_id: funded_channel.context.get_counterparty_node_id(), @@ -12412,7 +12397,7 @@ where }); } } else { - log_trace!(logger, "Sending channel_ready WITHOUT channel_update for {}", funded_channel.context.channel_id()); + log_trace!(logger, "Sending channel_ready WITHOUT channel_update for {}", channel_id); } }, #[cfg(splicing)] @@ -12423,7 +12408,7 @@ where let mut pending_events = self.pending_events.lock().unwrap(); pending_events.push_back((events::Event::ChannelReady { - channel_id: funded_channel.context.channel_id(), + channel_id: *channel_id, user_channel_id: funded_channel.context.get_user_id(), counterparty_node_id: funded_channel.context.get_counterparty_node_id(), funding_txo: funding_txo.map(|outpoint| outpoint.into_bitcoin_outpoint()), @@ -12495,8 +12480,8 @@ where // un-confirmed we force-close the channel, ensuring short_to_chan_info // is always consistent. let mut short_to_chan_info = self.short_to_chan_info.write().unwrap(); - let scid_insert = short_to_chan_info.insert(real_scid, (funded_channel.context.get_counterparty_node_id(), funded_channel.context.channel_id())); - assert!(scid_insert.is_none() || scid_insert.unwrap() == (funded_channel.context.get_counterparty_node_id(), funded_channel.context.channel_id()), + let scid_insert = short_to_chan_info.insert(real_scid, (funded_channel.context.get_counterparty_node_id(), *channel_id)); + assert!(scid_insert.is_none() || scid_insert.unwrap() == (funded_channel.context.get_counterparty_node_id(), *channel_id), "SCIDs should never collide - ensure you weren't behind by a full {} blocks when creating channels", fake_scid::MAX_SCID_BLOCKS_FROM_NOW); } @@ -12510,7 +12495,6 @@ where peer_state, err, funded_channel, - chan_id, FUNDED_CHANNEL ); failed_channels.push((Err(e), *counterparty_node_id)); diff --git a/lightning/src/ln/functional_tests.rs b/lightning/src/ln/functional_tests.rs index fa8f5ab5912..2faae7e1b4c 100644 --- a/lightning/src/ln/functional_tests.rs +++ b/lightning/src/ln/functional_tests.rs @@ -9067,7 +9067,7 @@ pub fn test_duplicate_chan_id() { } => { // Technically, at this point, nodes[1] would be justified in thinking both // channels are closed, but currently we do not, so we just move forward with it. - assert_eq!(msg.channel_id, channel_id); + assert_eq!(msg.channel_id, funding_created.temporary_channel_id); assert_eq!(node_id, node_a_id); }, _ => panic!("Unexpected event"), diff --git a/pending_changelog/3881.txt b/pending_changelog/3881.txt new file mode 100644 index 00000000000..ae9631f8ebc --- /dev/null +++ b/pending_changelog/3881.txt @@ -0,0 +1,6 @@ +API Updates +=========== + * Various instances of channel closure which provided a + `ClosureReason::HolderForceClosed` now provide more accurate + `ClosureReason`s, especially `ClosureReason::ProcessingError` (#3881). + * A new `ClosureReason::LocallyCoopClosedUnfundedChannel` was added (#3881).