Skip to content

Commit 0574bf8

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 92fc064 commit 0574bf8

File tree

1 file changed

+50
-58
lines changed

1 file changed

+50
-58
lines changed

‎lightning/src/ln/channelmanager.rs‎

Lines changed: 50 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -3242,6 +3242,23 @@ macro_rules! convert_channel_err {
32423242
},
32433243
}
32443244
};
3245+
($self: ident, $peer_state: expr, $shutdown_result: expr, $funded_channel: expr, $channel_id: expr, COOP_CLOSED) => { {
3246+
let reason = ChannelError::Close(("Coop Closed".to_owned(), $shutdown_result.closure_reason.clone()));
3247+
let do_close = |_| {
3248+
(
3249+
$shutdown_result,
3250+
$self.get_channel_update_for_broadcast(&$funded_channel).ok(),
3251+
)
3252+
};
3253+
let mut locked_close = |shutdown_res_mut: &mut ShutdownResult, funded_channel: &mut FundedChannel<_>| {
3254+
locked_close_channel!($self, $peer_state, funded_channel, shutdown_res_mut, FUNDED);
3255+
};
3256+
let (close, mut err) =
3257+
convert_channel_err!($self, $peer_state, reason, $funded_channel, do_close, locked_close, $channel_id, _internal);
3258+
err.dont_send_error_message();
3259+
debug_assert!(close);
3260+
(close, err)
3261+
} };
32453262
($self: ident, $peer_state: expr, $err: expr, $funded_channel: expr, $channel_id: expr, FUNDED_CHANNEL) => { {
32463263
let mut do_close = |reason| {
32473264
(
@@ -9312,13 +9329,14 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
93129329
msg.channel_id,
93139330
)
93149331
})?;
9315-
let (tx, chan_option, shutdown_result) = {
9332+
let logger;
9333+
let tx_err: Option<(_, Result<Infallible, _>)> = {
93169334
let mut peer_state_lock = peer_state_mutex.lock().unwrap();
93179335
let peer_state = &mut *peer_state_lock;
93189336
match peer_state.channel_by_id.entry(msg.channel_id.clone()) {
93199337
hash_map::Entry::Occupied(mut chan_entry) => {
93209338
if let Some(chan) = chan_entry.get_mut().as_funded_mut() {
9321-
let logger = WithChannelContext::from(&self.logger, &chan.context, None);
9339+
logger = WithChannelContext::from(&self.logger, &chan.context, None);
93229340
let res = chan.closing_signed(&self.fee_estimator, &msg, &&logger);
93239341
let (closing_signed, tx_shutdown_result) =
93249342
try_channel_entry!(self, peer_state, res, chan_entry);
@@ -9329,16 +9347,17 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
93299347
msg,
93309348
});
93319349
}
9332-
if let Some((tx, mut close_res)) = tx_shutdown_result {
9350+
if let Some((tx, close_res)) = tx_shutdown_result {
93339351
// We're done with this channel, we've got a signed closing transaction and
93349352
// will send the closing_signed back to the remote peer upon return. This
93359353
// also implies there are no pending HTLCs left on the channel, so we can
93369354
// fully delete it from tracking (the channel monitor is still around to
93379355
// watch for old state broadcasts)!
9338-
locked_close_channel!(self, peer_state, chan, close_res, FUNDED);
9339-
(Some(tx), Some(chan_entry.remove()), Some(close_res))
9356+
let (_, err) = convert_channel_err!(self, peer_state, close_res, chan, &msg.channel_id, COOP_CLOSED);
9357+
chan_entry.remove();
9358+
Some((tx, Err(err)))
93409359
} else {
9341-
(None, None, None)
9360+
None
93429361
}
93439362
} else {
93449363
return try_channel_entry!(self, peer_state, Err(ChannelError::close(
@@ -9348,26 +9367,11 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
93489367
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))
93499368
}
93509369
};
9351-
if let Some(broadcast_tx) = tx {
9352-
let channel_id = chan_option.as_ref().map(|channel| channel.context().channel_id());
9353-
log_info!(
9354-
WithContext::from(&self.logger, Some(*counterparty_node_id), channel_id, None),
9355-
"Broadcasting {}",
9356-
log_tx!(broadcast_tx)
9357-
);
9358-
self.tx_broadcaster.broadcast_transactions(&[&broadcast_tx]);
9359-
}
9360-
if let Some(chan) = chan_option.as_ref().and_then(Channel::as_funded) {
9361-
if let Ok(update) = self.get_channel_update_for_broadcast(chan) {
9362-
let mut pending_broadcast_messages =
9363-
self.pending_broadcast_messages.lock().unwrap();
9364-
pending_broadcast_messages
9365-
.push(MessageSendEvent::BroadcastChannelUpdate { msg: update });
9366-
}
9367-
}
93689370
mem::drop(per_peer_state);
9369-
if let Some(shutdown_result) = shutdown_result {
9370-
self.finish_close_channel(shutdown_result);
9371+
if let Some((broadcast_tx, err)) = tx_err {
9372+
log_info!(logger, "Broadcasting {}", log_tx!(broadcast_tx));
9373+
self.tx_broadcaster.broadcast_transactions(&[&broadcast_tx]);
9374+
let _ = handle_error!(self, err, *counterparty_node_id);
93719375
}
93729376
Ok(())
93739377
}
@@ -10462,12 +10466,6 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
1046210466
if let Some(broadcast_tx) = msgs.signed_closing_tx {
1046310467
log_info!(logger, "Broadcasting closing tx {}", log_tx!(broadcast_tx));
1046410468
self.tx_broadcaster.broadcast_transactions(&[&broadcast_tx]);
10465-
10466-
if let Ok(update) = self.get_channel_update_for_broadcast(&funded_chan) {
10467-
pending_msg_events.push(MessageSendEvent::BroadcastChannelUpdate {
10468-
msg: update
10469-
});
10470-
}
1047110469
}
1047210470
} else {
1047310471
// We don't know how to handle a channel_ready or signed_closing_tx for a
@@ -10481,40 +10479,45 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
1048110479
}
1048210480
};
1048310481

10484-
let mut shutdown_results = Vec::new();
10482+
let mut shutdown_results: Vec<(Result<Infallible, _>, _)> = Vec::new();
1048510483
let per_peer_state = self.per_peer_state.read().unwrap();
1048610484
let per_peer_state_iter = per_peer_state.iter().filter(|(cp_id, _)| {
1048710485
if let Some((counterparty_node_id, _)) = channel_opt {
1048810486
**cp_id == counterparty_node_id
1048910487
} else { true }
1049010488
});
10491-
for (_cp_id, peer_state_mutex) in per_peer_state_iter {
10489+
for (cp_id, peer_state_mutex) in per_peer_state_iter {
1049210490
let mut peer_state_lock = peer_state_mutex.lock().unwrap();
1049310491
let peer_state = &mut *peer_state_lock;
1049410492
peer_state.channel_by_id.retain(|_, chan| {
1049510493
let shutdown_result = match channel_opt {
1049610494
Some((_, channel_id)) if chan.context().channel_id() != channel_id => None,
1049710495
_ => unblock_chan(chan, &mut peer_state.pending_msg_events),
1049810496
};
10499-
if let Some(mut shutdown_result) = shutdown_result {
10497+
if let Some(shutdown_res) = shutdown_result {
1050010498
let context = chan.context();
1050110499
let logger = WithChannelContext::from(&self.logger, context, None);
10502-
log_trace!(logger, "Removing channel {} now that the signer is unblocked", context.channel_id());
10503-
if let Some(funded_channel) = chan.as_funded_mut() {
10504-
locked_close_channel!(self, peer_state, funded_channel, shutdown_result, FUNDED);
10500+
let chan_id = context.channel_id();
10501+
log_trace!(logger, "Removing channel {} now that the signer is unblocked", chan_id);
10502+
let (remove, err) = if let Some(funded_channel) = chan.as_funded_mut() {
10503+
convert_channel_err!(self, peer_state, shutdown_res, funded_channel, &chan_id, COOP_CLOSED)
1050510504
} else {
10506-
locked_close_channel!(self, chan.context(), UNFUNDED);
10507-
}
10508-
shutdown_results.push(shutdown_result);
10505+
debug_assert!(false);
10506+
let reason = shutdown_res.closure_reason.clone();
10507+
let err = ChannelError::Close((reason.to_string(), reason));
10508+
convert_channel_err!(self, peer_state, err, chan, &chan_id, UNFUNDED_CHANNEL)
10509+
};
10510+
debug_assert!(remove);
10511+
shutdown_results.push((Err(err), *cp_id));
1050910512
false
1051010513
} else {
1051110514
true
1051210515
}
1051310516
});
1051410517
}
1051510518
drop(per_peer_state);
10516-
for shutdown_result in shutdown_results.drain(..) {
10517-
self.finish_close_channel(shutdown_result);
10519+
for (err, counterparty_node_id) in shutdown_results {
10520+
let _ = handle_error!(self, err, counterparty_node_id);
1051810521
}
1051910522
}
1052010523

@@ -10525,11 +10528,10 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
1052510528
fn maybe_generate_initial_closing_signed(&self) -> bool {
1052610529
let mut handle_errors: Vec<(PublicKey, Result<(), _>)> = Vec::new();
1052710530
let mut has_update = false;
10528-
let mut shutdown_results = Vec::new();
1052910531
{
1053010532
let per_peer_state = self.per_peer_state.read().unwrap();
1053110533

10532-
for (_cp_id, peer_state_mutex) in per_peer_state.iter() {
10534+
for (cp_id, peer_state_mutex) in per_peer_state.iter() {
1053310535
let mut peer_state_lock = peer_state_mutex.lock().unwrap();
1053410536
let peer_state = &mut *peer_state_lock;
1053510537
let pending_msg_events = &mut peer_state.pending_msg_events;
@@ -10546,17 +10548,11 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
1054610548
});
1054710549
}
1054810550
debug_assert_eq!(tx_shutdown_result_opt.is_some(), funded_chan.is_shutdown());
10549-
if let Some((tx, mut shutdown_res)) = tx_shutdown_result_opt {
10550-
locked_close_channel!(self, peer_state, funded_chan, shutdown_res, FUNDED);
10551-
shutdown_results.push(shutdown_res);
10551+
if let Some((tx, shutdown_res)) = tx_shutdown_result_opt {
1055210552
// We're done with this channel. We got a closing_signed and sent back
1055310553
// a closing_signed with a closing transaction to broadcast.
10554-
if let Ok(update) = self.get_channel_update_for_broadcast(&funded_chan) {
10555-
let mut pending_broadcast_messages = self.pending_broadcast_messages.lock().unwrap();
10556-
pending_broadcast_messages.push(MessageSendEvent::BroadcastChannelUpdate {
10557-
msg: update
10558-
});
10559-
}
10554+
let (_, err) = convert_channel_err!(self, peer_state, shutdown_res, funded_chan, channel_id, COOP_CLOSED);
10555+
handle_errors.push((*cp_id, Err(err)));
1056010556

1056110557
log_info!(logger, "Broadcasting {}", log_tx!(tx));
1056210558
self.tx_broadcaster.broadcast_transactions(&[&tx]);
@@ -10577,14 +10573,10 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
1057710573
}
1057810574
}
1057910575

10580-
for (counterparty_node_id, err) in handle_errors.drain(..) {
10576+
for (counterparty_node_id, err) in handle_errors {
1058110577
let _ = handle_error!(self, err, counterparty_node_id);
1058210578
}
1058310579

10584-
for shutdown_result in shutdown_results.drain(..) {
10585-
self.finish_close_channel(shutdown_result);
10586-
}
10587-
1058810580
has_update
1058910581
}
1059010582

0 commit comments

Comments
 (0)