Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Fixed: Plugins: `keysend` now removes unknown even (technically illegal!) fields, to try to accept more payments.
The goal here is to test the node validation, not whether we can
trigger the schema validation with bogus values. So we bypass the
verifying RPC wrapper.
"sphinx_add_hop" takes a literal hop to include,
"sphinx_add_modern_hop" prepends the length. Now we always prepend a
length, make it clear that the literal version is a shortcut:
* sphinx_add_hop -> sphinx_add_hop_has_length
* sphinx_add_modern_hop -> sphinx_add_hop
In addition, we check that length is actually correct! This means
`createonion` can no longer create legacy or otherwise-invalid onions:
fix tests and update man page to remove legacy usage.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Changed: JSON-RPC: `createonion` no longer allows non-TLV-style payloads.
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.
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>
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>
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
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>
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>
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).
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)
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>
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>
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>
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>
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.
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>
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.
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`.
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>
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>
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`).
"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!)
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>
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>
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.
