Add a reason to HTLCHandlingFailed event #3601
Conversation
| /// The HTLC was failed by the local node. | ||
| Local { | ||
| /// The BOLT 04 failure code providing a specific failure reason. | ||
| failure_code: u16 |
There was a problem hiding this comment.
I admit I'm not super excited about exposing the failure code as-is for two reasons - (a) we have very different constraints on the code - we may not always want to give the sender all the details about a failure (eg if the sender used a private channel but we didn't mean to expose it we might pretend the channel doesnt exist) vs what we want to tell the user about a failure (basically everything), (b) its not particularly easy to parse - users have to go read the BOLT then map that to what is actually happening in LDK code, if we had a big enum here we'd be able to provide good docs linking to config knobs and reasons why failures happened.
There was a problem hiding this comment.
Makes sense, will take a look at adding a more detailed enum or adding to HTLCDestination 👍
| @@ -1441,6 +1461,10 @@ pub enum Event { | |||
| prev_channel_id: ChannelId, | |||
| /// Destination of the HTLC that failed to be processed. | |||
| failed_next_destination: HTLCDestination, | |||
| /// The cause of the processing failure. | |||
There was a problem hiding this comment.
Its worth noting that HTLCDestination has some "reason"-like attributes (I meant to mention this in the issue, but apparently forgot), they're just incomplete and too broad. Up to you if you want to try to add more details there vs adding a new enum.
|
I'm going to be out on vacation until 3 March, but will pick this up when I get back! |
This PR adds a
HTLCHandlingFailurereason toHTLCHandlingFailedto fix #3541.A simpler version of this would be to make
HTLCFailReasonpublic and just surface that on the API. Decided against this approach because it exposes not-so-user-friendly types on the API with types likeOnionErrorPacket, but happy to go with that way if it's preferred!