HTLC error cleanup #3472
HTLC error cleanup #3472
Commits on Feb 24, 2020
-
lightningd: rename htlc_in and htlc_out failuremsg fields to failonion.
This is clearer, especially when we also deal with raw not-yet-onion-wrapped failure messages. Signed-off-by: Rusty Russell <[email protected]>
-
lightningd: clean up weird call to send_htlc_out.
1. forward_htlc sets hout to NULL. 2. forward_htlc passes &hout to send_htlc_out. 3. forward_htlc checks the failcode and frees(NULL) and sets hout to NULL (again). This in fact covers every failcode which send_htlc_out returns. We should ensure send_htlc_out sets *houtp to NULL on failure; in fact, both callers pass houtp, so we can make it unconditional. Signed-off-by: Rusty Russell <[email protected]>
-
channeld: don't get details of our own failed htlcs at init.
For incoming htlcs, we need failure details in case we need to re-xmit them. But for outgoing htlcs, lightningd is telling us it already knows they've failed, so we just need to flag them failed and don't need the details. Internally, we set the ->fail to a dummy non-NULL value; this is cleaned up next. This matters for the next patch, which moves onion handling into lightningd. Signed-off-by: Rusty Russell <[email protected]>
-
lightningd: handle fail_htlc_in with no known outgoing channel.
Turn it into temporary node failure: this only happens if we restart with a failed htlc in, but it's clearer and more robust to handle it generically. Signed-off-by: Rusty Russell <[email protected]>
-
gossipd: provide (stripped) channel_update when resolving a channel.
I hadn't realized that lightningd asks gossipd every time we forward a payment. But I'm going to abuse it here to get the latest channel_update, otherwise (as lightningd takes over error message generation) lightningd needs to do an async request at various painful points. So have gossipd tell us the lastest update (stripped so compatible with the strange in-onion-error format). Signed-off-by: Rusty Russell <[email protected]>
-
channeld: get the onionreply back from lightningd for failed htlcs.
Instead of making it ourselves, lightningd does it. Now we only have two cases of failed htlcs: completely malformed (BADONION), and with an already-wrapped onion reply to send. This makes channeld's job much simpler. Signed-off-by: Rusty Russell <[email protected]>
-
lightningd: remove always-NULL argument to add_fail.
It's only called from the db code, and failing_channel is always NULL. Signed-off-by: Rusty Russell <[email protected]>
-
wallet: Add new htlc column "localfailmsg" for outgoing htlcs.
We're going to change our internal structure next, so this is preparation. We populate existing errors with temporary node failures, for simplicity. Signed-off-by: Rusty Russell <[email protected]>
-
channeld: add routing to get our own channel's channel_update.
We'll use this in the next patch for when we need to create errors to send back to lightningd; most commonly when the channel doesn't have capacity for the HTLC. Signed-off-by: Rusty Russell <[email protected]>
-
lightningd: store raw msg rather than code for locally-failed outgoin…
…g HTLCs At the moment, we store e.g. WIRE_TEMPORARY_CHANNEL_FAILURE, and then lightningd has a large demux function which turns that into the correct error message. Such an enum demuxer is an anti-pattern. Instead, store the message directly for output HTLCs; channeld now sends us an error message rather than an error code. For input HTLCs we will still need the failure code if the onion was bad (since we need to prompt channeld to send a completely different message than normal), though we can (and will!) eliminate its use in non-BADONION failure cases. Signed-off-by: Rusty Russell <[email protected]>
-
lightningd: separate path for failed_htlc when an onion is bad.
We tell channeld that an htlc is bad by sending it a 'struct failed_htlc'. This usually contains an onionreply to forward, but for the case where the onion itself was bad, it contains a failure code instead. This makes the "send a failed_htlc for a bad onion" a completely separate code path, then we can work on removing failcodes from the other path. In several places 'failcode' is now changed to 'badonion' to reflect that it can only be a BADONION failcode. Signed-off-by: Rusty Russell <[email protected]>
-
lightningd: make local htlc failures pass a wiremsg for errors, not a…
… failcode. Unfortunately the invoice_payment_hook can give us a failcode, so I simply restrict it to the two sensible ones. Signed-off-by: Rusty Russell <[email protected]> Changelog-deprecated: plugins: invoice_payment_hook "failure_code" only handles simple cases now, use "failure_message".
-
lightningd: always use an onionreply for locally generated incoming H…
…TLC errors (unless BADONION). This cleans up the "local failure" callers for incoming HTLCs to hand an onionreply instead of making us generate it from the code inside make_failmsg. (The db path still needs make_failmsg, so that's next). Signed-off-by: Rusty Russell <[email protected]> Changelog-deprecated: Plugins: htlc_accepted_hook "failure_code" only handles simple cases now, use "failure_message".
-
wallet: only store BADONION codes in db for incoming htlcs: rest are …
…all onionreplyies. This completes the conversion; any in-flight HTLC failures get turned into temporary_node_failures. Signed-off-by: Rusty Russell <[email protected]>
-
lightningd: rename htlc_in field from failcode to badonion.
That's all it's used for now. And remove unreferenced failoutchannel. Signed-off-by: Rusty Russell <[email protected]>
-
plugins: support failure_message in invoice and htlc_accepted hooks.
As promised in the Changelog when we converted from failcodes to messages internally. Signed-off-by: Rusty Russell <[email protected]>
