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

MPP: limit the number of HTLCs based on destination connectivity #3936

Merged
merged 5 commits into from
Sep 10, 2020
Merged

MPP: limit the number of HTLCs based on destination connectivity #3936

merged 5 commits into from
Sep 10, 2020

Commits on Sep 10, 2020

  1. pay: fix handling of legacy vs tlv encoding. 

    As revealed by the failure of tests in ElementsProject#3936, where we ended up trying
to send a partial payment using legacy style, we are not handling
style properly.

1. BOLT9 has features, so we can *know* that the destination supports
   MPP.  We may not have seen a node_announcement.
2. We can't assume that nodes inside routehints support TLV.
3. We can't assume direct peers support TLV.

The keysend code tried to fix this up, so I'm not sure that this caused
the issue in ElementsProject#3968, though.

Signed-off-by: Rusty Russell <[email protected]>
Changelog-Fixed: `pay` will now make reliable multi-part payments to nodes it doesn't have a node_announcement for.
    @rustyrussell
    rustyrussell committed Sep 10, 2020
    Configuration menu
    Copy the full SHA
    06cc838 View commit details
    Browse the repository at this point in the history

  2. tests/test_pay.py: Test for overload of number of HTLCs at the payee.

    @ZmnSCPxj @rustyrussell
    ZmnSCPxj authored and rustyrussell committed Sep 10, 2020
    Configuration menu
    Copy the full SHA
    0c5c504 View commit details
    Browse the repository at this point in the history

  3. plugins/libplugin-pay.c: Keep p->invoice->routes valid when the route… 

    …hints paymod mutates it.

The routehints paymod shares the storage of the array d->routehints and
p->invoice->routes, but once it operates, it possibly leaves it as a stale
pointer to memory it used to have.

Since other paymods may be interested in the invoice details, including
the routehints in the invoice, we should ensure the p->invoice->routes
remains valid whenever we try mutating that array.
    @ZmnSCPxj @rustyrussell
    ZmnSCPxj authored and rustyrussell committed Sep 10, 2020
    Configuration menu
    Copy the full SHA
    0e44180 View commit details
    Browse the repository at this point in the history

  4. plugins/libplugin-pay.c: Add facility to have paymods request lowerin… 

    …g of the estimated max HTLCs.
    @ZmnSCPxj @rustyrussell
    ZmnSCPxj authored and rustyrussell committed Sep 10, 2020
    Configuration menu
    Copy the full SHA
    cb5973c View commit details
    Browse the repository at this point in the history

  5. plugins/libplugin-pay.c: Add new payee_incoming_limit to limit number… 

    … of HTLCs based on payee connectivity.

Fixes: ElementsProject#3926

(probably)

Changelog-Fixed: pay: Also limit the number of splits if the payee seems to have a low number of channels that can enter it, given the max-concurrent-htlcs limit.
    @ZmnSCPxj @rustyrussell
    ZmnSCPxj authored and rustyrussell committed Sep 10, 2020
    Configuration menu
    Copy the full SHA
    08a22b7 View commit details
    Browse the repository at this point in the history