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

Fixes for HTLC db issues #2000

Merged
merged 21 commits into from Oct 9, 2018
Merged

Fixes for HTLC db issues #2000

merged 21 commits into from Oct 9, 2018

Commits on Oct 9, 2018

  1. pytest: make line_graph wait for gossip propagation if announce param… 

    … is True

This is what the callers want; generalize it.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
    @rustyrussell
    rustyrussell committed Oct 9, 2018
    Copy the full SHA
    9160ef1 View commit details
    Browse the repository at this point in the history

  2. Makefile: our first db-from-0.6.1 bugfix is coming. 

    Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
    @rustyrussell
    rustyrussell committed Oct 9, 2018
    Copy the full SHA
    3b7b61d View commit details
    Browse the repository at this point in the history

  3. lightningd: fix outstanding taken pointer. 

    lightningd: Outstanding taken pointers: lightningd/pay.c:243:channel_update

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
    @rustyrussell
    rustyrussell committed Oct 9, 2018
    Copy the full SHA
    6306021 View commit details
    Browse the repository at this point in the history

  4. lightningd: beef up HTLC consistency checking. 

    Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
    @rustyrussell
    rustyrussell committed Oct 9, 2018
    Copy the full SHA
    ba8c0c7 View commit details
    Browse the repository at this point in the history

  5. lightningd: even more HTLC consistency checking: check states. 

    This means we need to check when we've altered the state, so the checks
are moved to the callers of htlc_in_update_state and htlc_out_update_state.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
    @rustyrussell
    rustyrussell committed Oct 9, 2018
    Copy the full SHA
    ef10bae View commit details
    Browse the repository at this point in the history

  6. pytest: add test for when an incoming fulfilled HTLC expires before o… 

    …utgoing.

Usually, we only close an incoming HTLC once the outgoing HTLC is completely
resolved.  However, we short-cut this in the FULFILL case: we have the
preimage, so might as well use it immediately (in fact, we wait for it to
be committed, but we don't need to in theory).

As a side-effect of this, our assumption that every outgoing HTLC has
a corresponding incoming HTLC is incorrect, and this test (xfail) tickles
that corner case.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
    @rustyrussell
    rustyrussell committed Oct 9, 2018
    Copy the full SHA
    0503bb7 View commit details
    Browse the repository at this point in the history

  7. lightningd: handle case where incoming HTLC vanished before fulfilled… 

    … outgoing.

We now need an explicit 'local' flag, rather than relying on the existence
of the 'in' pointer.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
    @rustyrussell
    rustyrussell committed Oct 9, 2018
    Copy the full SHA
    ecbbdd9 View commit details
    Browse the repository at this point in the history

  8. lightningd: don't leave htlc_out's in pointer dangling when htlc_in f… 

    …reed.

Now we know this can happen (see previous patch), we need to handle it.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
    @rustyrussell
    rustyrussell committed Oct 9, 2018
    Copy the full SHA
    8188e88 View commit details
    Browse the repository at this point in the history

  9. pytest: extend the test_fulfill_incoming_first case to cover reconnect. 

    Which we don't handle, due to a separate bug, so it's xfail.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
    @rustyrussell
    rustyrussell committed Oct 9, 2018
    Copy the full SHA
    dd87cf2 View commit details
    Browse the repository at this point in the history

  10. lightningd: handle the case where the db contains a resolved HTLC wit… 

    …hout a preimage.

We need to handle this case (old db) before the next commit, which actually
fixes it.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
    @rustyrussell
    rustyrussell committed Oct 9, 2018
    Copy the full SHA
    553f90f View commit details
    Browse the repository at this point in the history

  11. lightningd: save outgoing HTLC's preimage to db. 

    We can now wrap the 'missing preimage' hack in COMPAT_V061.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
    @rustyrussell
    rustyrussell committed Oct 9, 2018
    Copy the full SHA
    f8b7ecc View commit details
    Browse the repository at this point in the history

  12. channeld: disable check for my_current_per_commitment_point. 

    Under stress, it fails (test_restart_many_payments, the next test).

I suspect a deep misunderstanding in the comparison code, will chase
separately.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
    @rustyrussell
    rustyrussell committed Oct 9, 2018
    Copy the full SHA
    e616a41 View commit details
    Browse the repository at this point in the history

  13. pytest: add restart-during-n-way payment test. 

    Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
    @rustyrussell
    rustyrussell committed Oct 9, 2018
    Copy the full SHA
    6f6b36c View commit details
    Browse the repository at this point in the history

  14. db: fix up HTLCs which are missing failure information. 

    We don't save them to the database, so fix things up as we load them.

Next patch will actually save them into the db, and this will become
COMPAT code.

Also: call htlc_in_check() with NULL on db load, as otherwise it aborts
internally.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
    @rustyrussell
    rustyrussell committed Oct 9, 2018
    Copy the full SHA
    8c3ef3d View commit details
    Browse the repository at this point in the history

  15. db: restore failuremsg/failcode from db. 

    We don't write it in there yet, so this change currently has no effect.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
    @rustyrussell
    rustyrussell committed Oct 9, 2018
    Copy the full SHA
    9d5072f View commit details
    Browse the repository at this point in the history

  16. db: save the failcode / failuremsg into db. 

    Now we can finally move the fixup code under COMPAT_V061, so it's only
for old nodes.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
    @rustyrussell
    rustyrussell committed Oct 9, 2018
    Copy the full SHA
    f1b213e View commit details
    Browse the repository at this point in the history

  17. lightningd: don't access htlc_in's failoutchannel on db restore. 

    failoutchannel tells us which channel to send an update for (specifically
for temporary_channel_failure); but we don't save it into the db.  It's
not even clear we should, since it's a corner case and the channel might
not even exist when we come back.

So on db restore, change such errors to WIRE_TEMPORARY_NODE_FAILURE
which doesn't need an update.

We also don't memset it to 0 in the normal case (we only access if it
failcode has the UPDATE bit set) so valgrind will trigger if we're
wrong.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
    @rustyrussell
    rustyrussell committed Oct 9, 2018
    Copy the full SHA
    b5b4929 View commit details
    Browse the repository at this point in the history

  18. lightningd: fix inconsistency without COMPAT enabled. 

    We don't expect payment or payment->route_channels to be NULL without an
old db, but putting an assert there reveals that we try to fail an HTLC
which has already succeeded in 'test_onchain_unwatch'.

Obviously we only want to fail an HTLC which goes onchain if we don't
already have the preimage!

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
    @rustyrussell
    rustyrussell committed Oct 9, 2018
    Copy the full SHA
    4da5970 View commit details
    Browse the repository at this point in the history

  19. htlc: rename local flag to am_origin, add FIXME. 

    Noted by @cdecker, the term 'local' is grossly overused, and the hout
preimage is basically only used as a sanity check (though I've just put
a FIXME there for now).

Also eliminated spurious blank line which crept into wallet.c.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
    @rustyrussell
    rustyrussell committed Oct 9, 2018
    Copy the full SHA
    d6b2cf2 View commit details
    Browse the repository at this point in the history

  20. pytest: make test_restart_many_payments more Pythony! 

    IIUC, namedtuple is like tuple for grown-ups: Pythonify!

Suggested-by: @cdecker
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
    @rustyrussell
    rustyrussell committed Oct 9, 2018
    Copy the full SHA
    8ad6142 View commit details
    Browse the repository at this point in the history

  21. CHANGELOG.md: update for flurry of HTLC fixes. 

    Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
    @rustyrussell
    rustyrussell committed Oct 9, 2018
    Copy the full SHA
    18b6c63 View commit details
    Browse the repository at this point in the history