Commit graph

2511 commits

Author SHA1 Message Date
Rusty Russell
c8ad9e18a9 common/onion: remove all trace of legacy parsing.
We still have an "enum forward_style" for the database, where old-style
forwards can still exist.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Removed: Protocol: we no longer forward HTLCs with legacy onions.
2022-09-28 13:40:57 +02:00
Rusty Russell
d4ef20d54a pytest: fix flake in test_gossip_persistence.
We used to ensure the l3<->l4 channel was private by
simply not mining enough blocks to announce.  Then
we started mining 13 blocks to close the channel, and
it will get announced, causing a failure:

```
        assert non_public(l2) == []
>       wait_for(lambda: non_public(l3) == [scid34, scid34])
...
>               raise ValueError("Timeout while waiting for {}", success)
E               ValueError: ('Timeout while waiting for {}', <function test_gossip_persistence.<locals>.<lambda> at 0x7f6cc69b4170>)
```

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2022-09-27 13:14:20 +02:00
Rusty Russell
695f001161 pytest: fix flake in test_zeroconf_forward
Second pay can fail if first is not completely settled:

```
    def test_zeroconf_forward(node_factory, bitcoind):
        """Ensure that we can use zeroconf channels in forwards.

...
        # Make sure (esp in non-dev-mode) blockheights agree so we don't WIRE_EXPIRY_TOO_SOON...
        sync_blockheight(bitcoind, [l1, l2, l3])
        inv = l3.rpc.invoice(42 * 10**6, 'inv1', 'desc')['bolt11']
        l1.rpc.pay(inv)

        # And now try the other way around: zeroconf channel first
        # followed by a public one.
        wait_for(lambda: len(l3.rpc.listchannels()['channels']) == 4)
        inv = l1.rpc.invoice(42, 'back1', 'desc')['bolt11']
>       l3.rpc.pay(inv)

...
>           raise RpcError(method, payload, resp['error'])
E           pyln.client.lightning.RpcError: RPC call failed: method: pay, payload: {'bolt11': 'lnbcrt420p1p3junrssp588gtjzmlrr4pfj7ssmdlulzhhushrpq3rdqxjuz2m33scsvzdjlspp5fk5yhu6netc0d0sgp8es52vjk6akhd3uayr08u8max4d8rwzpjuqdq8v3jhxccxqyjw5qcqp99qyysgqcpyyugejv4nya97v6gw8fhtr0ru3vq87jjlltav99wlat436a95n0z8yzdp699p9md0zz9tmnsjpvfj622n9g9fh7r6ldhpgh9wmr4qpcru3rk'}, error: {'code': 210, 'message': 'Destination 0266e4598d1d3c415f572a8488830b60f7e744ed9235eb0b1ba93283b315c03518 is not reachable directly and all routehints were unusable.', 'attempts': [{'status': 'failed', 'failreason': 'Destination 0266e4598d1d3c415f572a8488830b60f7e744ed9235eb0b1ba93283b315c03518 is not reachable directly and all routehints were unusable.', 'partid': 0, 'amount_msat': 42msat}]}
```

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2022-09-27 13:14:20 +02:00
Rusty Russell
68f15f17bb delforward: allow deletion of "unknown in_htlc_id" and fix autoclean to use it.
Note the caveats: we will delete *all* of them at once!

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2022-09-27 14:42:03 +09:30
Rusty Russell
6eac8dfe3c delforward: tally up deleted forwards so that getinfo's fees_collected_msat doesn't change.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Fixes: #5627
2022-09-27 14:42:03 +09:30
Rusty Russell
cafa1a8c65 db: correctly migrate forwards for closed incoming channels.
We have to allow them (as otherwise `fees_collected_msat` in getinfo breaks),
but it means that actually, in_htlc_id might be missing in listforwards
(also, out_htlc_id might be missing, which we didn't catch before).

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Fixes: #5628
2022-09-27 14:42:03 +09:30
Rusty Russell
9023bd9334 pytest: add test for migrations upgrade which breaks 'fees_collected_msat'.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2022-09-27 14:42:03 +09:30
Rusty Russell
6e86fa9220 lightningd: figure out optimal channel *before* forward_htlc hook.
Otherwise what the hook sees is actually a lie, and if it sets it
we might override it.

The side effect is that we add an explicit "forward_to" field, and
allow hooks to override it.  This lets a *hook* control channel
choice explicitly.

Changelod-Added: Plugins: `htlc_accepted_hook` return can specify what channel to forward htlc to.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2022-09-26 13:52:04 +02:00
Rusty Russell
b698a5a5ef channeld: send error, not warning, if peer has old commitment number.
This is the minimal change to meet the desired outcome of https://github.com/lightning/bolts/issues/934
which wants to give obsolete-db nodes a chance to fix things up, before we
close the channel.

We need to dance around a bit here, since we *will* close the channel if
we receive an ERROR, so we suppress that.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2022-09-26 11:36:09 +02:00
Rusty Russell
a1f62ba0e7 gossipd: don't close non-local channels immediately, add 12 block delay.
This adds a new "chan_dying" message to the gossip_store, but since we
already changed the minor version in this PR, we don't bump it again.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Added: Protocol: We now delay forgetting funding-spent channels for 12 blocks (as per latest BOLTs, to support splicing in future).
2022-09-24 15:22:27 +09:30
Rusty Russell
253b25522b BOLT: update to version which requires option_channel_htlc_max.
We will now simply reject old-style ones as invalid.  Turns out the
only trace we could find is a channel between two nodes unconnected to
the rest of the network.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Changed: Protocol: We now require all channel_update messages include htlc_maximum_msat (as per latest BOLTs)
2022-09-24 15:22:27 +09:30
Rusty Russell
daa5269ea2 gossipd: bump gossip_store to indicate all channel_update have htlc_max.
And in the next patch, gossipd will no longer put new ones in.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2022-09-24 15:22:27 +09:30
Rusty Russell
fe556d1ed9 gossipd: don't try to upgrade ancient gossip_store.
If they really upgrade directly from 0.9.2, it will simply delete the
store and re-fetch it.

We still update from v9 (which could be v0.11), since it's a noop.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2022-09-24 15:22:27 +09:30
Rusty Russell
9be6ed6236 pytest: fix flake in test_pay_disconnect
If channeld hasn't exited yet, it's possible we'll send the message (we would fail later, in
waitsendpay, but just not immediately).  So wait for that explicitly.

```
2022-09-22T22:49:59.6737296Z         with pytest.raises(RpcError, match=r'failed: WIRE_TEMPORARY_CHANNEL_FAILURE \(First peer not ready\)'):
2022-09-22T22:49:59.6737566Z >           l1.rpc.sendpay(route, rhash, payment_secret=inv['payment_secret'])
2022-09-22T22:49:59.6737865Z E           Failed: DID NOT RAISE <class 'pyln.client.lightning.RpcError'>
2022-09-22T22:49:59.6737873Z 
```

And from the listpeers output, ou can see "connected" false, but owner channeld:

```
2022-09-22T22:49:59.7493163Z DEBUG:root:{
2022-09-22T22:49:59.7493320Z   "id": "-c:listpeers#26",
2022-09-22T22:49:59.7493397Z   "result": {
2022-09-22T22:49:59.7493477Z     "peers": [
2022-09-22T22:49:59.7493548Z       {
2022-09-22T22:49:59.7493709Z         "id": "022d223620a359a47ff7f7ac447c85c46c923da53389221a0054c11c1e3ca31d59",
2022-09-22T22:49:59.7493801Z         "connected": false,
2022-09-22T22:49:59.7493884Z         "channels": [
2022-09-22T22:49:59.7493955Z           {
2022-09-22T22:49:59.7494058Z             "state": "CHANNELD_NORMAL",
2022-09-22T22:49:59.7494250Z             "scratch_txid": "4b95a3b1b5e1a970401a169a3697f3a9bfbfbcb59d3d21434aa1f3fb2980db8d",
2022-09-22T22:49:59.7494365Z             "last_tx_fee_msat": "7965000msat",
2022-09-22T22:49:59.7494437Z             "feerate": {
2022-09-22T22:49:59.7494529Z               "perkw": 11000,
2022-09-22T22:49:59.7494618Z               "perkb": 44000
2022-09-22T22:49:59.7494690Z             },
2022-09-22T22:49:59.7494785Z             "owner": "channeld",
2022-09-22T22:49:59.7494894Z             "short_channel_id": "103x1x0",
```

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2022-09-23 14:40:16 -05:00
Rusty Russell
57002f3381 pytest: fix flake in test_onchain_different_fees
Sometimes, we haven't reconnected, and so the peer does not exist at all after
the channel is forgotten:

```
2022-09-22T22:49:59.3985374Z         # Now, 100 blocks it should be done.
2022-09-22T22:49:59.3985656Z         bitcoind.generate_block(100)
2022-09-22T22:49:59.3986112Z >       wait_for(lambda: only_one(l1.rpc.listpeers()['peers'])['channels'] == [])
2022-09-22T22:49:59.3986338Z 
2022-09-22T22:49:59.3986523Z tests/test_closing.py:2715: 
2022-09-22T22:49:59.3986810Z _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
2022-09-22T22:49:59.3987241Z contrib/pyln-testing/pyln/testing/utils.py:90: in wait_for
2022-09-22T22:49:59.3987568Z     while not success():
2022-09-22T22:49:59.3987917Z tests/test_closing.py:2715: in <lambda>
2022-09-22T22:49:59.3988389Z     wait_for(lambda: only_one(l1.rpc.listpeers()['peers'])['channels'] == [])
2022-09-22T22:49:59.3988737Z _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
2022-09-22T22:49:59.3988908Z 
2022-09-22T22:49:59.3988979Z arr = []
2022-09-22T22:49:59.3989106Z 
2022-09-22T22:49:59.3989209Z     def only_one(arr):
2022-09-22T22:49:59.3989545Z         """Many JSON RPC calls return an array; often we only expect a single entry
2022-09-22T22:49:59.3989849Z         """
2022-09-22T22:49:59.3990063Z >       assert len(arr) == 1
2022-09-22T22:49:59.3990388Z E       AssertionError
```

You can see it's empty from the call here:

```
2022-09-22T22:49:59.6697941Z DEBUG:root:{
2022-09-22T22:49:59.6698106Z   "id": "-c:listpeers#42",
2022-09-22T22:49:59.6698179Z   "result": {
2022-09-22T22:49:59.6698270Z     "peers": []
2022-09-22T22:49:59.6698346Z   }
2022-09-22T22:49:59.6698422Z }
```

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2022-09-23 14:40:16 -05:00
Rusty Russell
651753bbd5 pytest: slow down test_autoclean.
CI is really slow: it sees all three expire at once.  But making the
timeouts too long is painful in non-VALGRIND, so I ended up making it
conditional.

```
         # First it expires.
        wait_for(lambda: only_one(l3.rpc.listinvoices('inv1')['invoices'])['status'] == 'expired')
        # Now will get autocleaned
        wait_for(lambda: l3.rpc.listinvoices('inv1')['invoices'] == [])
>       assert l3.rpc.autoclean_status()['autoclean']['expiredinvoices']['cleaned'] == 1
E       assert 3 == 1

tests/test_plugin.py:2975: AssertionError
```

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2022-09-22 15:19:46 +02:00
Rusty Russell
f52ff07558 lightningd: allow delpay to delete a specific payment.
This is actually what the autoclean plugin wants, especially since
you can't otherwise delete a payment which has failed then succeeded.

But insist on neither or both being specified, at least for now.

Changelog-Added: JSON-RPC: `delpay` takes optional `groupid` and `partid` parameters to specify exactly what payment to delete.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2022-09-22 15:19:46 +02:00
Rusty Russell
13e10877de autoclean: add autoclean-once command.
Changelog-Added: Plugins: `autoclean-once` command for a single cleanup.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2022-09-22 15:19:46 +02:00
Rusty Russell
399288db3f autoclean: use config variables, not commands.
It's more natural: we will eventually support dynamic config variables,
so this will be quite nice.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Added: Plugins: `autoclean` can now delete old forwards, payments, and invoices automatically.
2022-09-22 15:19:46 +02:00
Rusty Russell
a15f1be5f8 autoclean: clean up listforwards as well.
And take the opportunity to rename l0 and l1 in the tests to the
more natural l1 l2 (since we add l3).

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2022-09-22 15:19:46 +02:00
Rusty Russell
3079afb024 lightningd: add delforward command.
Changelog-Added: JSON-RPC: `delforward` command to delete listforwards entries.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2022-09-22 15:19:46 +02:00
Rusty Russell
7420a7021f lightningd: add listhtlcs to list all the HTLCs we know about.
Using `listfowards` for this wrong; expose this directly if people
care (and unlike listforwards, which could be deleted, we have to
remember these while the channel is still open!).

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Added: JSON-RPC: `listhtlcs` new command to list all known HTLCS.
2022-09-22 15:19:46 +02:00
Rusty Russell
311807ff1f lightningd: add in_htlc_id / out_htlc_id to listforwards.
And document that we never know payment_hash.

Changelog-Added: JSON-RPC: `listforwards` now shows `in_htlc_id` and `out_htlc_id`
Changelog-Changed: JSON-RPC: `listforwards` now never shows `payment_hash`; use `listhtlcs`.
2022-09-22 15:19:46 +02:00
Rusty Russell
d7c1325e38 wallet: use scid not string for failchannel (now failscid) in payments table.
And remove the now-unused string-based helper functions.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2022-09-22 15:19:46 +02:00
Rusty Russell
2752e04f8f db: add scid field to channels table.
Normally, we'd use the delete_columns function to remove the old
`short_channel_id` string field, *but* we can't do that for sqlite, as
there are other tables with references to it.  So add a FIXME to do
it once everyone has upgraded to an sqlite3 which has native support
for column deletion.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2022-09-22 15:19:46 +02:00
Rusty Russell
63457229cb wallet: replace forwarded_payments table with forwards table.
This one directly contains the scids of the channels involved, not
references, so can outlive the channels.  As a side-effect, however,
it now never lists `payment_hash`.  Having it listed (via join) is not
possible as it is a *string* in the channels table, and difficult
anyway because of channel aliases.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2022-09-22 15:19:46 +02:00
Rusty Russell
4cab396cc8 autoclean: handle cleaning of old payments (not just invoices).
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2022-09-22 15:19:46 +02:00
Rusty Russell
660c9af1d9 autoclean: allow cleaning of paid invoices too.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2022-09-22 15:19:46 +02:00
Rusty Russell
7da51892e8 autoclean: save stats on how much we cleaned.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2022-09-22 15:19:46 +02:00
Rusty Russell
17858c9490 lightningd: deprecated "delexpiredinvoice", put functionality in autoclean plugin.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Deprecated: JSON-RPC: `delexpiredinvoice`: use `autoclean-once`.
2022-09-22 15:19:46 +02:00
Rusty Russell
bd76a196f5 autoclean: new interface
In preparation for more things being autocleaned.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2022-09-22 15:19:46 +02:00
Rusty Russell
0868fa9f1e lightningd: allow extra tlv types in non-experimental mode.
The old `experimental-accept-extra-tlv-types` is now `accept-htlc-tlv-types`.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Added: Config: `accept-htlc-tlv-types` lets us accept unknown even HTLC TLV fields we would normally reject on parsing (was EXPERIMENTAL-only `experimental-accept-extra-tlv-types`).
2022-09-22 17:19:11 +09:30
Rusty Russell
df4b477e88 keysend: allow extratlvs parameter, even in non-experimental mode.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Added: JSON-RPC: `keysend` now has `extratlvs` option in non-EXPERIMENTAL builds.
2022-09-22 17:19:11 +09:30
Rusty Russell
ce0f544073 keysend: try to find description in TLV.
"Who needs specs?"  FFS...

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Added: Protocol: `keysend` will now attach the longest valid text field in the onion to the invoice (so you can have Sphinx.chat users spam you!)
2022-09-22 17:19:11 +09:30
Rusty Russell
0db01c882f pytest: fix flake in test_sendcustommsg
We assume that because we've told l3 to shut down, l2 already sees it
as disconnected.  But CI is ...slow... today!

```
        # `l3` is disconnected and we can't send messages to it
>       assert(not l2.rpc.listpeers(l3.info['id'])['peers'][0]['connected'])
E       assert not True

tests/test_misc.py:2218: AssertionError
```

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2022-09-21 12:15:48 +02:00
Christian Decker
774d16a72e openingd: Fail if dust and max_htlcs result in 0output commitment tx
Prior to this we might end up with a commitment transaction without
any outputs, if combined with `--dev-allowdustreserve`. Otherwise the
reserve being larger than dust means the funder could not drop its
direct output to be below dust.

Reported-by: Rusty Russell <@rustyrussell>
2022-09-21 11:25:47 +02:00
Christian Decker
493a0dfcd4 pytest: Exercise all dust zeroreserve case
This just shows we need to add that constraint.

Reported-by: Rusty Russell <@rustyrussell>
2022-09-21 11:25:47 +02:00
Christian Decker
bdda62e1a4 pytest: Add test for mixed zeroreserve funding
Tests that we can have zeroreserve nodes accept and open new channels
with non-zeroreserve nodes, without throwing errors
2022-09-21 11:25:47 +02:00
Christian Decker
54b4baabb0 opening: Add dev-allowdustreserve option to opt into dust reserves
Technically this is a non-conformance with the spec, hence the `dev`
flag to opt-in, however I'm being told that it is also implemented in
other implementations. I'll follow this up with a proposal to the spec
to remove the checks we now bypass.
2022-09-21 11:25:47 +02:00
Christian Decker
c5b2aee5c6 pytest: Add a zeroreserve test 2022-09-21 11:25:47 +02:00
Rusty Russell
3380f559f9 memleak: simplify API.
Mainly renaming.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2022-09-19 11:34:42 +09:30
Rusty Russell
caecd1ee0a lightningd: don't log JSON ids as debug, use log io.
They are cute, sure, but they do spam the logs.

@Suggested-by: @niftynei
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2022-09-16 12:31:45 +09:30
Rusty Russell
f1f2c1322d contrib/pyln-client: construct JSON ID correctly.
They can set their name explicitly, but if they don't we extract it from argv[0].

We also set it around callbacks, so it will be expanded by default.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2022-09-16 12:31:45 +09:30
Rusty Russell
d360075d22 libplugin: use string ids correctly.
Build them from the command which caused them, and take plugin name
as basename with extension stripped.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2022-09-16 12:31:45 +09:30
Rusty Russell
e8ef42b741 plugin: wire JSON id for commands which caused hooks to fire.
Most obvious one is the "connect" hook.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2022-09-16 12:31:45 +09:30
Rusty Russell
eceb9f4328 lightningd: wire plugin command JSON id through to plugin commands.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2022-09-16 12:31:45 +09:30
Rusty Russell
ea7903f69a lightningd: trace JSON id prefixes through sendrawtx.
First, merge the _ahf_ and non-ahf interfaces.
Second, remove the always-NULL txs->cmd field.

Then, add optional id_prefix for bitcoind_sendrawx, so if it's
triggered by a command (e.g. "withdraw") it's shown correctly in logs.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2022-09-16 12:31:45 +09:30
Rusty Russell
a9557d5194 lightningd: derive JSONRPC ids from incoming id (append /cln:<method>#NNN).
Usually the calls are spontanous, so it's just "cln:<method>#NNN", but
json_invoice() calls listincoming, and json_checkmessage calls
listnodes, so those become "cli:invoice-<pid>/cln:listincoming#NNN".

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2022-09-16 12:31:45 +09:30
Rusty Russell
8fcf880e0f lightningd: explicitly remember if JSON id was a string.
This lets us use 'cmd->id' as an unquoted string (for building
new ids!).

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2022-09-16 12:31:45 +09:30
Rusty Russell
ed3f700991 lightningd: use string as json req ids when we create them.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2022-09-16 12:31:45 +09:30