Simplify channelmanager handle_error via map_err
What changed, and why it matters
This commit is a straightforward code cleanup in the Lightning Dev Kit's channel manager. It rewrites an error-handling function to use Rust's `map_err` helper instead of an explicit `match` on `Ok`/`Err`. The behavior is unchanged: the same channel-closing logic, logging, and peer-message generation runs. No security issue is present.
No security action needed. Treat as a normal refactoring commit.
Security signals we found
No strong security signals were identified.
Evidence from the diff
The patch refactors ChannelManager::handle_error from an explicit match internal { Ok(msg) => ..., Err(e) => ... } into internal.map_err(|err_internal| { ...; err_internal.err }). All side effects (logging, finish_close_channel, pending broadcast messages, peer pending message events) are preserved inside the closure. The function signature, return type, and observable behavior remain identical. A doc comment was also added.
Changed components
lightning/src/ln/channelmanager.rsInspect captured patch +46 / −48
diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs
index 460994e..222fcf8 100644
--- a/lightning/src/ln/channelmanager.rs
+++ b/lightning/src/ln/channelmanager.rs
@@ -3968,6 +3968,7 @@ where
}
}
+ /// Handles an error by closing the channel if required and generating peer messages.
fn handle_error<A>(
&self, internal: Result<A, MsgHandleErrInternal>, counterparty_node_id: PublicKey,
) -> Result<A, LightningError> {
@@ -3976,64 +3977,61 @@ where
debug_assert_ne!(self.pending_events.held_by_thread(), LockHeldState::HeldByThread);
debug_assert_ne!(self.per_peer_state.held_by_thread(), LockHeldState::HeldByThread);
- match internal {
- Ok(msg) => Ok(msg),
- Err(MsgHandleErrInternal { err, shutdown_finish, tx_abort, .. }) => {
- let mut msg_event = None;
+ internal.map_err(|err_internal| {
+ let mut msg_event = None;
- if let Some((shutdown_res, update_option)) = shutdown_finish {
- let counterparty_node_id = shutdown_res.counterparty_node_id;
- let channel_id = shutdown_res.channel_id;
- let logger = WithContext::from(
- &self.logger,
- Some(counterparty_node_id),
- Some(channel_id),
- None,
- );
- log_error!(logger, "Closing channel: {}", err.err);
-
- self.finish_close_channel(shutdown_res);
- if let Some((update, node_id_1, node_id_2)) = update_option {
- let mut pending_broadcast_messages =
- self.pending_broadcast_messages.lock().unwrap();
- pending_broadcast_messages.push(MessageSendEvent::BroadcastChannelUpdate {
- msg: update,
- node_id_1,
- node_id_2,
- });
- }
- } else {
- log_error!(self.logger, "Got non-closing error: {}", err.err);
+ if let Some((shutdown_res, update_option)) = err_internal.shutdown_finish {
+ let counterparty_node_id = shutdown_res.counterparty_node_id;
+ let channel_id = shutdown_res.channel_id;
+ let logger = WithContext::from(
+ &self.logger,
+ Some(counterparty_node_id),
+ Some(channel_id),
+ None,
+ );
+ log_error!(logger, "Closing channel: {}", err_internal.err.err);
+
+ self.finish_close_channel(shutdown_res);
+ if let Some((update, node_id_1, node_id_2)) = update_option {
+ let mut pending_broadcast_messages =
+ self.pending_broadcast_messages.lock().unwrap();
+ pending_broadcast_messages.push(MessageSendEvent::BroadcastChannelUpdate {
+ msg: update,
+ node_id_1,
+ node_id_2,
+ });
}
+ } else {
+ log_error!(self.logger, "Got non-closing error: {}", err_internal.err.err);
+ }
- if let msgs::ErrorAction::IgnoreError = err.action {
- if let Some(tx_abort) = tx_abort {
- msg_event = Some(MessageSendEvent::SendTxAbort {
- node_id: counterparty_node_id,
- msg: tx_abort,
- });
- }
- } else {
- msg_event = Some(MessageSendEvent::HandleError {
+ if let msgs::ErrorAction::IgnoreError = err_internal.err.action {
+ if let Some(tx_abort) = err_internal.tx_abort {
+ msg_event = Some(MessageSendEvent::SendTxAbort {
node_id: counterparty_node_id,
- action: err.action.clone(),
+ msg: tx_abort,
});
}
+ } else {
+ msg_event = Some(MessageSendEvent::HandleError {
+ node_id: counterparty_node_id,
+ action: err_internal.err.action.clone(),
+ });
+ }
- if let Some(msg_event) = msg_event {
- let per_peer_state = self.per_peer_state.read().unwrap();
- if let Some(peer_state_mutex) = per_peer_state.get(&counterparty_node_id) {
- let mut peer_state = peer_state_mutex.lock().unwrap();
- if peer_state.is_connected {
- peer_state.pending_msg_events.push(msg_event);
- }
+ if let Some(msg_event) = msg_event {
+ let per_peer_state = self.per_peer_state.read().unwrap();
+ if let Some(peer_state_mutex) = per_peer_state.get(&counterparty_node_id) {
+ let mut peer_state = peer_state_mutex.lock().unwrap();
+ if peer_state.is_connected {
+ peer_state.pending_msg_events.push(msg_event);
}
}
+ }
- // Return error in case higher-API need one
- Err(err)
- },
- }
+ // Return error in case higher-API need one
+ err_internal.err
+ })
}
/// Gets the current [`UserConfig`] which controls some global behavior and includes the
Why this scored 15/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.