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

Reproduce pay route crash #3633

Merged
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
2 changes: 1 addition & 1 deletion doc/lightning-invoice.7

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion doc/lightning-invoice.7.md
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ logic, which will use unpublished channels only if there are no
published channels. If *true* unpublished channels are always considered
as a route hint candidate; if *false*, never. If it is a short channel id
(e.g. *1x1x3*) or array of short channel ids, only those specific channels
will be considered candidates, even if they are public.
will be considered candidates, even if they are public or dead-ends.

The route hint is selected from the set of incoming channels of which:
peer’s balance minus their reserves is at least *msatoshi*, state is
Expand Down
6 changes: 5 additions & 1 deletion lightningd/invoice.c
Original file line number Diff line number Diff line change
Expand Up @@ -640,7 +640,11 @@ static void gossipd_incoming_channels_reply(struct subd *gossipd,
&inchans[i].short_channel_id)) {
tal_arr_remove(&inchans, i);
tal_arr_remove(&inchan_deadends, i);
}
i--;
} else
/* If they specify directly, we don't
* care if it's a deadend */
inchan_deadends[i] = false;
}

/* If they told us to use scids and we couldn't, fail. */
Expand Down
26 changes: 22 additions & 4 deletions tests/test_invoices.py
Original file line number Diff line number Diff line change
Expand Up @@ -284,13 +284,13 @@ def test_invoice_routeboost_private(node_factory, bitcoind):
assert r['cltv_expiry_delta'] == 6

# Ask it explicitly to use a channel it can't (insufficient capacity)
inv = l2.rpc.invoice(msatoshi=10, label="inv5", description="?", exposeprivatechannels=scid2)
assert 'warning_deadends' in inv
assert 'warning_capacity' not in inv
inv = l2.rpc.invoice(msatoshi=(10**5) * 1000 + 1, label="inv5", description="?", exposeprivatechannels=scid2)
assert 'warning_deadends' not in inv
assert 'warning_capacity' in inv
assert 'warning_offline' not in inv

# Give it two options and it will pick one with suff capacity.
inv = l2.rpc.invoice(msatoshi=10, label="inv6", description="?", exposeprivatechannels=[scid2, scid])
inv = l2.rpc.invoice(msatoshi=(10**5) * 1000 + 1, label="inv6", description="?", exposeprivatechannels=[scid2, scid])
assert 'warning_capacity' not in inv
assert 'warning_offline' not in inv
assert 'warning_deadends' not in inv
Expand All @@ -302,6 +302,24 @@ def test_invoice_routeboost_private(node_factory, bitcoind):
assert r['fee_proportional_millionths'] == 10
assert r['cltv_expiry_delta'] == 6

# It will use an explicit exposeprivatechannels even if it thinks its a dead-end
l0.rpc.close(l1.info['id'])
l0.wait_for_channel_onchain(l1.info['id'])
bitcoind.generate_block(1)
wait_for(lambda: l2.rpc.listchannels(scid_dummy)['channels'] == [])

inv = l2.rpc.invoice(msatoshi=123456, label="inv7", description="?", exposeprivatechannels=scid)
assert 'warning_capacity' not in inv
assert 'warning_offline' not in inv
assert 'warning_deadends' not in inv
# Route array has single route with single element.
r = only_one(only_one(l1.rpc.decodepay(inv['bolt11'])['routes']))
assert r['pubkey'] == l1.info['id']
assert r['short_channel_id'] == l1.rpc.listchannels()['channels'][0]['short_channel_id']
assert r['fee_base_msat'] == 1
assert r['fee_proportional_millionths'] == 10
assert r['cltv_expiry_delta'] == 6


def test_invoice_expiry(node_factory, executor):
l1, l2 = node_factory.line_graph(2, fundchannel=True)
Expand Down
17 changes: 17 additions & 0 deletions tests/test_pay.py
Original file line number Diff line number Diff line change
Expand Up @@ -2942,3 +2942,20 @@ def test_sendpay_blinding(node_factory):
payment_hash=inv['payment_hash'],
bolt11=inv['bolt11'])
l1.rpc.waitsendpay(inv['payment_hash'])


def test_excluded_adjacent_routehint(node_factory, bitcoind):
"""Test case where we try have a routehint which leads to an adjacent
node, but the result exceeds our maxfee; we crashed trying to find
what part of the path was most expensive in that case

"""
l1, l2, l3 = node_factory.line_graph(3)

# We'll be forced to use routehint, since we don't know about l3.
wait_for(lambda: l3.channel_state(l2) == 'CHANNELD_NORMAL')
inv = l3.rpc.invoice(10**3, "lbl", "desc", exposeprivatechannels=l2.get_channel_scid(l3))

# This will make it reject the routehint.
with pytest.raises(RpcError, match=r'Route wanted fee of 1msat'):
l1.rpc.pay(bolt11=inv['bolt11'], maxfeepercent=0, exemptfee=0)