Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

HTLC error cleanup #3472

Merged
merged 16 commits into from
Feb 25, 2020
Merged

HTLC error cleanup #3472

merged 16 commits into from
Feb 25, 2020

Commits on Feb 24, 2020

  1. 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 <rusty@rustcorp.com.au>
    @rustyrussell
    rustyrussell committed Feb 24, 2020
    Configuration menu
    Copy the full SHA
    36f93ce View commit details
    Browse the repository at this point in the history

  2. 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 <rusty@rustcorp.com.au>
    @rustyrussell
    rustyrussell committed Feb 24, 2020
    Configuration menu
    Copy the full SHA
    9d8bc21 View commit details
    Browse the repository at this point in the history

  3. 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 <rusty@rustcorp.com.au>
    @rustyrussell
    rustyrussell committed Feb 24, 2020
    Configuration menu
    Copy the full SHA
    45308b8 View commit details
    Browse the repository at this point in the history

  4. 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 <rusty@rustcorp.com.au>
    @rustyrussell
    rustyrussell committed Feb 24, 2020
    Configuration menu
    Copy the full SHA
    832a5c2 View commit details
    Browse the repository at this point in the history

  5. 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 <rusty@rustcorp.com.au>
    @rustyrussell
    rustyrussell committed Feb 24, 2020
    Configuration menu
    Copy the full SHA
    c9f5dc2 View commit details
    Browse the repository at this point in the history

  6. 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 <rusty@rustcorp.com.au>
    @rustyrussell
    rustyrussell committed Feb 24, 2020
    Configuration menu
    Copy the full SHA
    6d0f6f0 View commit details
    Browse the repository at this point in the history

  7. 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 <rusty@rustcorp.com.au>
    @rustyrussell
    rustyrussell committed Feb 24, 2020
    Configuration menu
    Copy the full SHA
    6fab127 View commit details
    Browse the repository at this point in the history

  8. 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 <rusty@rustcorp.com.au>
    @rustyrussell
    rustyrussell committed Feb 24, 2020
    Configuration menu
    Copy the full SHA
    5b7d83f View commit details
    Browse the repository at this point in the history

  9. 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 <rusty@rustcorp.com.au>
    @rustyrussell
    rustyrussell committed Feb 24, 2020
    Configuration menu
    Copy the full SHA
    8a18d9f View commit details
    Browse the repository at this point in the history

  10. 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 <rusty@rustcorp.com.au>
    @rustyrussell
    rustyrussell committed Feb 24, 2020
    Configuration menu
    Copy the full SHA
    eb5eeb6 View commit details
    Browse the repository at this point in the history

  11. 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 <rusty@rustcorp.com.au>
    @rustyrussell
    rustyrussell committed Feb 24, 2020
    Configuration menu
    Copy the full SHA
    ea23f9b View commit details
    Browse the repository at this point in the history

  12. 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 <rusty@rustcorp.com.au>
Changelog-deprecated: plugins: invoice_payment_hook "failure_code" only handles simple cases now, use "failure_message".
    @rustyrussell
    rustyrussell committed Feb 24, 2020
    Configuration menu
    Copy the full SHA
    82613d4 View commit details
    Browse the repository at this point in the history

  13. 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 <rusty@rustcorp.com.au>
Changelog-deprecated: Plugins: htlc_accepted_hook "failure_code" only handles simple cases now, use "failure_message".
    @rustyrussell
    rustyrussell committed Feb 24, 2020
    Configuration menu
    Copy the full SHA
    953a583 View commit details
    Browse the repository at this point in the history

  14. 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 <rusty@rustcorp.com.au>
    @rustyrussell
    rustyrussell committed Feb 24, 2020
    Configuration menu
    Copy the full SHA
    8bf6393 View commit details
    Browse the repository at this point in the history

  15. 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 <rusty@rustcorp.com.au>
    @rustyrussell
    rustyrussell committed Feb 24, 2020
    Configuration menu
    Copy the full SHA
    c07e1aa View commit details
    Browse the repository at this point in the history

  16. 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 <rusty@rustcorp.com.au>
    @rustyrussell
    rustyrussell committed Feb 24, 2020
    Configuration menu
    Copy the full SHA
    4dbeac8 View commit details
    Browse the repository at this point in the history