Simplify ChannelUnavailable APIError handling with let-else
What the developer wrote
Authored by Abeeujah
Simplify ChannelUnavailable APIError handling with let-else
Refactor the nested match statement used during error construction into
a more idiomatic let-else construct (stabilised in Rust 1.65). The
previous implementation required verbose, nested matching to navigate
around borrow checker limitations.
By leveraging let-else alongside chaining unwrap_err on handle_error
Result, we achieve the same teardown logic (dropping state locks)
and error mapping with significantly less boilerplate and nesting.
What changed, and why it matters
This is a pure code cleanup that rewrites an awkward nested match statement into a newer, more idiomatic Rust construct called let-else. It does not change what the program does, what data it accepts, or how it responds to errors. There is no security-relevant behavior change.
No security action needed. Treat as normal code-quality review.
Security signals we found
No strong security signals were identified.
Evidence from the diff
The commit refactors error handling in ChannelManager around a handle_error call. Previously a verbose match on res plus an inner match on the always-Err Result from handle_error was used to work around borrow-checker constraints. The patch replaces that with a let-else binding and unwrap_err() on the handle_error Result. The teardown drops (peer_state_lock, per_peer_state) and the returned APIError::ChannelUnavailable remain identical. No functional change.
Changed components
lightning/src/ln/channelmanager.rsInspect captured patch +5 / −20
diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs
index 4939226..123d26d 100644
--- a/lightning/src/ln/channelmanager.rs
+++ b/lightning/src/ln/channelmanager.rs
@@ -11465,26 +11465,11 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
},
};
- // We have to match below instead of map_err on the above as in the map_err closure the borrow checker
- // would consider peer_state moved even though we would bail out with the `?` operator.
- let (channel_id, mut channel, message_send_event) = match res {
- Ok(res) => res,
- Err(err) => {
- mem::drop(peer_state_lock);
- mem::drop(per_peer_state);
- // TODO(dunxen): Find/make less icky way to do this.
- match self.handle_error(
- Result::<(), MsgHandleErrInternal>::Err(err),
- *counterparty_node_id,
- ) {
- Ok(_) => {
- unreachable!("`handle_error` only returns Err as we've passed in an Err")
- },
- Err(e) => {
- return Err(APIError::ChannelUnavailable { err: e.err });
- },
- }
- },
+ let Ok((channel_id, mut channel, message_send_event)) = res else {
+ mem::drop(peer_state_lock);
+ mem::drop(per_peer_state);
+ let e = self.handle_error::<()>(res.map(|_| ()), *counterparty_node_id).unwrap_err();
+ return Err(APIError::ChannelUnavailable { err: e.err });
};
if trusted_channel_features.is_some_and(|f| f.is_0conf()) {
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.