Use substitude channel for forwarding if we have another with the same peer

[TheBlueMatt]

I believe we're now the only major lightning implementation that insists on forwarding HTLCs out over the same channel as described in the SCID in the onion. Instead, all other implementations will happily forward over a different channel if another one is available to the same counterparty node and it doesn't have enough liquidity in the first channel. We should do the same.

[jurvis]

hi @ViktorTigerstrom, does your PR #1325 address this issue?

[ViktorTigerstrom]

Hi @jurvis, no not really. My PR does not deal with forwarding HTCLs at all.

The only way my PR could be seen as slightly related to this is that when our implementation is used to generate the invoice, the hints generally won't include channels with too low inbound capacity, so that this scenario hopefully shouldn't occur at end of the path (meaning the sender hopefully shouldn't construct a route for the last hop in which the 2nd last node will face this problem). But that's stretching it a bit :)

[jurvis]

@ViktorTigerstrom ah okay -- I saw that this issue was tagged in #1279 so I wanted to clarify. thank you!

[ViktorTigerstrom]

I'd be interested in trying to this one up, unless you think this one is too big to tackle at the moment.
It would be fun to work on something a bit more substantial, and to get into the HTLC handling!

Before I dig too deep into this, I just have a few questions which would be really helpful to get an answer to:

Do I understand the issue correctly, that we should forward the HTLC over another channel with the same counterparty node (if a channel with enough outbound balance exists) if send_htlc fails here due too low outbound balance in the channel?

rust-lightning/lightning/src/ln/channelmanager.rs

Line 2987 in 29727a3

match chan.get_mut().send_htlc(amt_to_forward, payment_hash, outgoing_cltv_value, htlc_source.clone(), onion_packet, &self.logger) {

Do I also understand it correctly, that in order to do that in reasonably effective way, we would need to update the ChannelManager::per_peer_state to hold the Channels per counterparty node, so that the other channels for the same counterparty node could be found easily?

We'd also need to refactor the code in the if clause here, so that it uses the correct channel for the corresponding add_htlc_msgs if another channel was used to forward the payment:

rust-lightning/lightning/src/ln/channelmanager.rs

Line 3049 in 29727a3

if !add_htlc_msgs.is_empty() || !fail_htlc_msgs.is_empty() {

Are there other aspects to this issue that complicates it further?

[TheBlueMatt]

Instead of doing this all at once, probably best to break it into small chunks and make progress over time. eg if you first redo the storage of channels, try tackling #105 as a side-effect of doing so then coming back to this one.

[ViktorTigerstrom]

Ok thanks, sounds like a good idea! I'll start by looking into that :)