Skip to content

Commit 5b2e68e

Browse files
TheBlueMattTheBlueMatt
committed
Drop the remaining direct uses of locked_close_channel
While most of our closure logic mostly lives in `locked_close_chan`, some important steps happen in `convert_channel_err` and `handle_error` (mostly broadcasting a channel closure `ChannelUpdate` and sending a relevant error message to our peer). In a previous commit we removed most instances of `locked_close_channel` in our closure pipeline, however it was still called in cases where we did a cooperative close. Here we remove the remaining direct references to it, always using `convert_channel_err` instead (now with a new `COOP_CLOSED` macro variant).
1 parent e238ac0 commit 5b2e68e

File tree

1 file changed

+52
-59
lines changed

1 file changed

+52
-59
lines changed

‎lightning/src/ln/channelmanager.rs‎

Lines changed: 52 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -3069,7 +3069,7 @@ macro_rules! handle_error {
30693069
let logger = WithContext::from(
30703070
&$self.logger, Some(counterparty_node_id), Some(channel_id), None
30713071
);
3072-
log_error!(logger, "Force-closing channel: {}", err.err);
3072+
log_error!(logger, "Closing channel: {}", err.err);
30733073

30743074
$self.finish_close_channel(shutdown_res);
30753075
if let Some(update) = update_option {
@@ -3192,6 +3192,23 @@ macro_rules! convert_channel_err {
31923192
},
31933193
}
31943194
};
3195+
($self: ident, $peer_state: expr, $shutdown_result: expr, $funded_channel: expr, $channel_id: expr, COOP_CLOSED) => { {
3196+
let reason = ChannelError::Close(("Coop Closed".to_owned(), $shutdown_result.closure_reason.clone()));
3197+
let do_close = |_| {
3198+
(
3199+
$shutdown_result,
3200+
$self.get_channel_update_for_broadcast(&$funded_channel).ok(),
3201+
)
3202+
};
3203+
let mut locked_close = |shutdown_res_mut: &mut ShutdownResult, funded_channel: &mut FundedChannel<_>| {
3204+
locked_close_channel!($self, $peer_state, funded_channel, shutdown_res_mut, FUNDED);
3205+
};
3206+
let (close, mut err) =
3207+
convert_channel_err!($self, $peer_state, reason, $funded_channel, do_close, locked_close, $channel_id, _internal);
3208+
err.dont_send_error_message();
3209+
debug_assert!(close);
3210+
err
3211+
} };
31953212
($self: ident, $peer_state: expr, $err: expr, $funded_channel: expr, $channel_id: expr, FUNDED_CHANNEL) => { {
31963213
let mut do_close = |reason| {
31973214
(
@@ -9788,13 +9805,14 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
97889805
msg.channel_id,
97899806
)
97909807
})?;
9791-
let (tx, chan_option, shutdown_result) = {
9808+
let logger;
9809+
let tx_err: Option<(_, Result<Infallible, _>)> = {
97929810
let mut peer_state_lock = peer_state_mutex.lock().unwrap();
97939811
let peer_state = &mut *peer_state_lock;
97949812
match peer_state.channel_by_id.entry(msg.channel_id.clone()) {
97959813
hash_map::Entry::Occupied(mut chan_entry) => {
97969814
if let Some(chan) = chan_entry.get_mut().as_funded_mut() {
9797-
let logger = WithChannelContext::from(&self.logger, &chan.context, None);
9815+
logger = WithChannelContext::from(&self.logger, &chan.context, None);
97989816
let res = chan.closing_signed(&self.fee_estimator, &msg, &&logger);
97999817
let (closing_signed, tx_shutdown_result) =
98009818
try_channel_entry!(self, peer_state, res, chan_entry);
@@ -9805,16 +9823,17 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
98059823
msg,
98069824
});
98079825
}
9808-
if let Some((tx, mut close_res)) = tx_shutdown_result {
9826+
if let Some((tx, close_res)) = tx_shutdown_result {
98099827
// We're done with this channel, we've got a signed closing transaction and
98109828
// will send the closing_signed back to the remote peer upon return. This
98119829
// also implies there are no pending HTLCs left on the channel, so we can
98129830
// fully delete it from tracking (the channel monitor is still around to
98139831
// watch for old state broadcasts)!
9814-
locked_close_channel!(self, peer_state, chan, close_res, FUNDED);
9815-
(Some(tx), Some(chan_entry.remove()), Some(close_res))
9832+
let err = convert_channel_err!(self, peer_state, close_res, chan, &msg.channel_id, COOP_CLOSED);
9833+
chan_entry.remove();
9834+
Some((tx, Err(err)))
98169835
} else {
9817-
(None, None, None)
9836+
None
98189837
}
98199838
} else {
98209839
return try_channel_entry!(self, peer_state, Err(ChannelError::close(
@@ -9824,26 +9843,11 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
98249843
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))
98259844
}
98269845
};
9827-
if let Some(broadcast_tx) = tx {
9828-
let channel_id = chan_option.as_ref().map(|channel| channel.context().channel_id());
9829-
log_info!(
9830-
WithContext::from(&self.logger, Some(*counterparty_node_id), channel_id, None),
9831-
"Broadcasting {}",
9832-
log_tx!(broadcast_tx)
9833-
);
9834-
self.tx_broadcaster.broadcast_transactions(&[&broadcast_tx]);
9835-
}
9836-
if let Some(chan) = chan_option.as_ref().and_then(Channel::as_funded) {
9837-
if let Ok(update) = self.get_channel_update_for_broadcast(chan) {
9838-
let mut pending_broadcast_messages =
9839-
self.pending_broadcast_messages.lock().unwrap();
9840-
pending_broadcast_messages
9841-
.push(MessageSendEvent::BroadcastChannelUpdate { msg: update });
9842-
}
9843-
}
98449846
mem::drop(per_peer_state);
9845-
if let Some(shutdown_result) = shutdown_result {
9846-
self.finish_close_channel(shutdown_result);
9847+
if let Some((broadcast_tx, err)) = tx_err {
9848+
log_info!(logger, "Broadcasting {}", log_tx!(broadcast_tx));
9849+
self.tx_broadcaster.broadcast_transactions(&[&broadcast_tx]);
9850+
let _ = handle_error!(self, err, *counterparty_node_id);
98479851
}
98489852
Ok(())
98499853
}
@@ -11045,12 +11049,6 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
1104511049
if let Some(broadcast_tx) = msgs.signed_closing_tx {
1104611050
log_info!(logger, "Broadcasting closing tx {}", log_tx!(broadcast_tx));
1104711051
self.tx_broadcaster.broadcast_transactions(&[&broadcast_tx]);
11048-
11049-
if let Ok(update) = self.get_channel_update_for_broadcast(&funded_chan) {
11050-
pending_msg_events.push(MessageSendEvent::BroadcastChannelUpdate {
11051-
msg: update
11052-
});
11053-
}
1105411052
}
1105511053
} else {
1105611054
// We don't know how to handle a channel_ready or signed_closing_tx for a
@@ -11064,40 +11062,46 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
1106411062
}
1106511063
};
1106611064

11067-
let mut shutdown_results = Vec::new();
11065+
let mut shutdown_results: Vec<(Result<Infallible, _>, _)> = Vec::new();
1106811066
let per_peer_state = self.per_peer_state.read().unwrap();
1106911067
let per_peer_state_iter = per_peer_state.iter().filter(|(cp_id, _)| {
1107011068
if let Some((counterparty_node_id, _)) = channel_opt {
1107111069
**cp_id == counterparty_node_id
1107211070
} else { true }
1107311071
});
11074-
for (_cp_id, peer_state_mutex) in per_peer_state_iter {
11072+
for (cp_id, peer_state_mutex) in per_peer_state_iter {
1107511073
let mut peer_state_lock = peer_state_mutex.lock().unwrap();
1107611074
let peer_state = &mut *peer_state_lock;
1107711075
peer_state.channel_by_id.retain(|_, chan| {
1107811076
let shutdown_result = match channel_opt {
1107911077
Some((_, channel_id)) if chan.context().channel_id() != channel_id => None,
1108011078
_ => unblock_chan(chan, &mut peer_state.pending_msg_events),
1108111079
};
11082-
if let Some(mut shutdown_result) = shutdown_result {
11080+
if let Some(shutdown) = shutdown_result {
1108311081
let context = chan.context();
1108411082
let logger = WithChannelContext::from(&self.logger, context, None);
11085-
log_trace!(logger, "Removing channel {} now that the signer is unblocked", context.channel_id());
11086-
if let Some(funded_channel) = chan.as_funded_mut() {
11087-
locked_close_channel!(self, peer_state, funded_channel, shutdown_result, FUNDED);
11083+
let chan_id = context.channel_id();
11084+
log_trace!(logger, "Removing channel {} now that the signer is unblocked", chan_id);
11085+
let (remove, err) = if let Some(funded) = chan.as_funded_mut() {
11086+
let err = convert_channel_err!(self, peer_state, shutdown, funded, &chan_id, COOP_CLOSED);
11087+
(true, err)
1108811088
} else {
11089-
locked_close_channel!(self, chan.context(), UNFUNDED);
11090-
}
11091-
shutdown_results.push(shutdown_result);
11089+
debug_assert!(false);
11090+
let reason = shutdown.closure_reason.clone();
11091+
let err = ChannelError::Close((reason.to_string(), reason));
11092+
convert_channel_err!(self, peer_state, err, chan, &chan_id, UNFUNDED_CHANNEL)
11093+
};
11094+
debug_assert!(remove);
11095+
shutdown_results.push((Err(err), *cp_id));
1109211096
false
1109311097
} else {
1109411098
true
1109511099
}
1109611100
});
1109711101
}
1109811102
drop(per_peer_state);
11099-
for shutdown_result in shutdown_results.drain(..) {
11100-
self.finish_close_channel(shutdown_result);
11103+
for (err, counterparty_node_id) in shutdown_results {
11104+
let _ = handle_error!(self, err, counterparty_node_id);
1110111105
}
1110211106
}
1110311107

@@ -11108,11 +11112,10 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
1110811112
fn maybe_generate_initial_closing_signed(&self) -> bool {
1110911113
let mut handle_errors: Vec<(PublicKey, Result<(), _>)> = Vec::new();
1111011114
let mut has_update = false;
11111-
let mut shutdown_results = Vec::new();
1111211115
{
1111311116
let per_peer_state = self.per_peer_state.read().unwrap();
1111411117

11115-
for (_cp_id, peer_state_mutex) in per_peer_state.iter() {
11118+
for (cp_id, peer_state_mutex) in per_peer_state.iter() {
1111611119
let mut peer_state_lock = peer_state_mutex.lock().unwrap();
1111711120
let peer_state = &mut *peer_state_lock;
1111811121
let pending_msg_events = &mut peer_state.pending_msg_events;
@@ -11129,17 +11132,11 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
1112911132
});
1113011133
}
1113111134
debug_assert_eq!(tx_shutdown_result_opt.is_some(), funded_chan.is_shutdown());
11132-
if let Some((tx, mut shutdown_res)) = tx_shutdown_result_opt {
11133-
locked_close_channel!(self, peer_state, funded_chan, shutdown_res, FUNDED);
11134-
shutdown_results.push(shutdown_res);
11135+
if let Some((tx, shutdown_res)) = tx_shutdown_result_opt {
1113511136
// We're done with this channel. We got a closing_signed and sent back
1113611137
// a closing_signed with a closing transaction to broadcast.
11137-
if let Ok(update) = self.get_channel_update_for_broadcast(&funded_chan) {
11138-
let mut pending_broadcast_messages = self.pending_broadcast_messages.lock().unwrap();
11139-
pending_broadcast_messages.push(MessageSendEvent::BroadcastChannelUpdate {
11140-
msg: update
11141-
});
11142-
}
11138+
let err = convert_channel_err!(self, peer_state, shutdown_res, funded_chan, channel_id, COOP_CLOSED);
11139+
handle_errors.push((*cp_id, Err(err)));
1114311140

1114411141
log_info!(logger, "Broadcasting {}", log_tx!(tx));
1114511142
self.tx_broadcaster.broadcast_transactions(&[&tx]);
@@ -11160,14 +11157,10 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
1116011157
}
1116111158
}
1116211159

11163-
for (counterparty_node_id, err) in handle_errors.drain(..) {
11160+
for (counterparty_node_id, err) in handle_errors {
1116411161
let _ = handle_error!(self, err, counterparty_node_id);
1116511162
}
1116611163

11167-
for shutdown_result in shutdown_results.drain(..) {
11168-
self.finish_close_channel(shutdown_result);
11169-
}
11170-
1117111164
has_update
1117211165
}
1117311166

0 commit comments

Comments
 (0)