Allow specifying an error when failing back

[TheBlueMatt]

When we fail back because the user tells us to, we should let the user specify the error message we send to the peer.

[alecchendev]

Hi! I’m interested in picking up this issue.

From what I understand, this would involve changing the fail_htlc_backwards(&self, payment_hash: &PaymentHash) function on ChannelManager? Particularly letting the user provide another input that could affect the HTLCFailReason that’s passed into fail_htlc_backwards_internal(..)?

I imagine just adding an argument to the function is probably not the best idea for backwards compatibility, in which case would it be better to add a new function, something along the lines of fail_htlc_backwards_with_reason(&self, payment_hash: &PaymentHash, reason: &HTLCFailReason)?

I also wanted to ask for a brief comprehension check on my understanding of how failing back an HTLC works in the ChannelManager. From digging through the codebase today, this is what I think happens:

• A payment is sent to the user, which emits an Event::PaymentClaimable.
• Usually the user would then call ChannelManager::claim_funds, but if the preimage for the payment hash is unknown and they can’t claim the payment, or something else is wrong like the amount received is wrong, the user will fail back the HTLC by calling this function, ChannelManager::fail_htlc_backwards.
• This function goes through each source for the payment (there are multiple in case of a multi-path payment?) and calls ChannelManager::fail_htlc_backwards_internal, notably with an HTLCSource::PreviousHopData, and an HTLCFailReason.
• This function now adds an HTLCForwardInfo::FailHTLC (with the fail reason as an OnionErrorPacket) to self.forward_htlcs, and emits an Event::PendingHTLCsForwardable, and Event::HTLCHandlingFailed.
• The user is then expected to handle Event::PendingHTLCsForwardable, which includes calling ChannelManager::process_pending_htlc_forwards, which does a lot of things, but importantly loops through self.forward_htlcs and if the channel exists, for HTLCForwardInfo::FailHTLC, it calls Channel::queue_fail_htlc, which calls Channel::fail_htlc which actually fails it. I couldn’t find where it actually sends a message to a peer, but that’s about as far as I got.

Any feedback is appreciated!

[valentinewallace]

something along the lines of fail_htlc_backwards_with_reason(&self, payment_hash: &PaymentHash, reason: &HTLCFailReason)?

That looks accurate to me. And your comprehension about how HTLCs are failed back looks good too!

[alecchendev]

Cool! I'll probably open a PR sometime today or tomorrow.