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.

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

Routehints get ignored if invoice >10,000 sats #3908

Merged
merged 2 commits into from
Aug 6, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 17 additions & 1 deletion plugins/pay.c
Original file line number Diff line number Diff line change
Expand Up @@ -1881,9 +1881,25 @@ struct payment_modifier *paymod_mods[] = {
&local_channel_hints_pay_mod,
&exemptfee_pay_mod,
&directpay_pay_mod,
&presplit_pay_mod,
&shadowroute_pay_mod,
/* NOTE: The order in which these two paymods are executed is
* significant!
* routehints *must* execute first before presplit.
*
* FIXME: Giving an ordered list of paymods to the paymod
* system is the wrong interface, given that the order in
* which paymods execute is significant.
* (This is typical of Entity-Component-System pattern.)
* What should be done is that libplugin-pay should have a
* canonical list of paymods in the order they execute
* correctly, and whether they are default-enabled/default-disabled,
* and then clients like `pay` and `keysend` will disable/enable
* paymods that do not help them, instead of the current interface
* where clients provide an *ordered* list of paymods they want to
* use.
*/
&routehints_pay_mod,
&presplit_pay_mod,
&waitblockheight_pay_mod,
&retry_pay_mod,
&adaptive_splitter_pay_mod,
Expand Down
28 changes: 28 additions & 0 deletions tests/test_pay.py
Original file line number Diff line number Diff line change
Expand Up @@ -3221,3 +3221,31 @@ def test_bolt11_null_after_pay(node_factory, bitcoind):
pays = l2.rpc.listpays()["pays"]
assert(pays[0]["bolt11"] == invl1)
assert('amount_msat' in pays[0] and pays[0]['amount_msat'] == amt)


def test_mpp_presplit_routehint_conflict(node_factory, bitcoind):
'''
We had a bug where pre-splitting the payment prevents *any*
routehints from being taken.
We tickle that bug here by building l1->l2->l3, but with
l2->l3 as an unpublished channel.
If the payment is large enough to trigger pre-splitting, the
routehints are not applied in any of the splits.
'''
l1, l2, l3 = node_factory.get_nodes(3)

l1.rpc.connect(l2.info['id'], 'localhost', l2.port)
l1l2 = l1.fund_channel(l2, 10**7, announce_channel=True)
l2.rpc.connect(l3.info['id'], 'localhost', l3.port)
l2.fund_channel(l3, 10**7, announce_channel=False)

bitcoind.generate_block(6)
sync_blockheight(bitcoind, [l1, l2, l3])

# Wait for l3 to learn about l1->l2, otherwise it will think
# l2 is a deadend and not add it to the routehint.
wait_for(lambda: len(l3.rpc.listchannels(l1l2)['channels']) >= 2)

inv = l3.rpc.invoice(Millisatoshi(2 * 10000 * 1000), 'i', 'i', exposeprivatechannels=True)['bolt11']

l1.rpc.pay(inv)