Commit Graph

11059 Commits

Author SHA1 Message Date
Rusty Russell e36d4d1143 devtools/decodemsg: fix printing of wireaddr.
printwire_ routines are supposed to print!  And they're only needed
inside devtools/.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2022-03-25 13:55:44 +10:30
Rusty Russell 6f7d51ee68 common/onion: always fill in failtlvtype and failtlvpos parameters.
When we did fill them in, we filled them in wrong: the offset should be
the offset in the message, not the field number!

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2022-03-25 13:55:44 +10:30
Rusty Russell 613a21693d test: avoid testing for secret_eq under CI.
It's failing under CI, but loadavg is under 1.  Simply remove it.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2022-03-25 13:40:55 +10:30
Sjors Provoost 560b090bd8 Add more warnings to fundchannel_start
[ Slight modifications to fix schema -- RR ]
2022-03-24 14:11:40 +10:30
Rusty Russell f95c9522fd plugins/bcli: fix false memleak detection.
Well, it is leaking a bool for the command duration, but that's probably OK.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2022-03-24 13:19:10 +10:30
Rusty Russell f7aba31448 lightningd: allow 'style' 'legacy' for clboss compat (with deprecated_apis).
We keep the parameter around for future use (e.g. PTLC support?), but clboss
master still sets 'style' to "legacy".  Accept, but ignore it.

Reported-by: grubman on #c-lightning (IRC)
Changelog-Deprecated: JSON-RPC: `sendpay` `route` argument `style` "legacy" (don't use it at all, we ignore it now and always use "tlv" anyway).
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2022-03-24 13:19:10 +10:30
Rusty Russell 5aa108f176 pytest: fix flake in test_pay.py::test_setchannel_state
The second disconnect could fail, if slow enough:

```
>       l1.rpc.disconnect(l2.info['id'], force=True)

tests/test_pay.py:2067:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
contrib/pyln-client/pyln/client/lightning.py:690: in disconnect
    return self.call("disconnect", payload)
contrib/pyln-testing/pyln/testing/utils.py:639: in call
    res = LightningRpc.call(self, method, payload)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

self = <pyln.testing.utils.PrettyPrintingLightningRpc object at 0x7fb139ce87d0>
method = 'disconnect'
payload = {'force': True, 'id': '035d2b1192dfba134e10e540875d366ebc8bc353d5aa766b80c090b39c3a5d885d'}

...
>           raise RpcError(method, payload, resp['error'])
E           pyln.client.lightning.RpcError: RPC call failed: method: disconnect, payload: {'id': '035d2b1192dfba134e10e540875d366ebc8bc353d5aa766b80c090b39c3a5d885d', 'force': True}, error: {'code': -1, 'message': 'Peer not connected'}
```

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2022-03-24 09:53:38 +10:30
Vincenzo Palazzo 53806d1abd cli: make the command line more user friendly.
Also has to fix up tests.

Changelog-Fixed: cli doesn't required anymore to confirm the password if the `hsm_secret` is already encrypted.

Signed-off-by: Vincenzo Palazzo <vincenzopalazzodev@gmail.com>
2022-03-24 09:53:38 +10:30
Vincenzo Palazzo 58ce4cf71c cli: check if the pass and the confirmation pass match
Changelog-Fixed: check if the pass and the confirmation pass match from the command line

Signed-off-by: Vincenzo Palazzo <vincenzopalazzodev@gmail.com>
2022-03-24 09:53:38 +10:30
Rusty Russell be75518559 pytest: fix bad gossip flake in test_multifunding_v1_v2_mixed
This was missed in e8d2176e6b.

```
>           raise ValueError(str(errors))
E           ValueError: 
E           Node errors:
E            - lightningd-2: had bad gossip messages
E            - lightningd-3: had bad gossip messages
E           Global errors:

contrib/pyln-testing/pyln/testing/fixtures.py:201: ValueError

...

0266e4598d1d3c415f572a8488830b60f7e744ed9235eb0b1ba93283b315c03518-gossipd: Ignoring future channel_announcment for 105x1x2 (current block 104)
0266e4598d1d3c415f572a8488830b60f7e744ed9235eb0b1ba93283b315c03518-gossipd: Bad gossip order: WIRE_CHANNEL_UPDATE before announcement 105x1x2/0
```

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2022-03-23 13:20:12 +10:30
Rusty Russell 9c978f2c63 lightningd: free peer->uc when openingd fails.
This avoids reuse which can cause confusion; the long-term fix is to
rewrite this to use real channels like dualfunding does.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2022-03-23 13:20:12 +10:30
Rusty Russell bcf3cef96c pytest: fix flake in test_connection.py::test_opener_feerate_reconnect
Make sure it sees disconnect before reconnect, otherwise the next command
fails since we're now disconnected.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2022-03-23 13:20:12 +10:30
Rusty Russell 2a80400a0f pytest: fix test_connection.py::test_funding_close_upfront fake
Breaks when there are no peers:

```
>       _fundchannel(l1, l2, amt_normal, None)

tests/test_connection.py:1564: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
tests/test_connection.py:1535: in _fundchannel
    wait_for(lambda: not has_normal_channels(l2, l1))
contrib/pyln-testing/pyln/testing/utils.py:88: in wait_for
    while not success():
tests/test_connection.py:1535: in <lambda>
    wait_for(lambda: not has_normal_channels(l2, l1))
tests/test_connection.py:1527: in has_normal_channels
    for c in only_one(l1.rpc.listpeers(l2.info['id'])['peers'])['channels']])
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

arr = []

    def only_one(arr):
        """Many JSON RPC calls return an array; often we only expect a single entry
        """
>       assert len(arr) == 1
E       AssertionError
```

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2022-03-23 13:20:12 +10:30
Rusty Russell d2cf2a3f51 gossipd: don't use connectd daemon_conn after it shuts down.
Simply exit, like we do when master daemon_conn exits.

```
Valgrind error file: valgrind-errors.2211908
==2211908== Invalid read of size 8
==2211908==    at 0x12AC13: daemon_conn_send (daemon_conn.c:137)
==2211908==    by 0x113CD9: queue_peer_msg (gossipd.c:118)
==2211908==    by 0x11B806: query_channel_range (queries.c:1169)
==2211908==    by 0x1250DD: peer_gossip_probe_scids (seeker.c:706)
==2211908==    by 0x1253B1: check_firstpeer (seeker.c:788)
==2211908==    by 0x1256CA: seeker_check (seeker.c:884)
==2211908==    by 0x1366AC: timer_expired (timeout.c:62)
==2211908==    by 0x1163D1: main (gossipd.c:1146)
==2211908==  Address 0x4cafdf0 is 48 bytes inside a block of size 88 free'd
==2211908==    at 0x48460C4: free (vg_replace_malloc.c:872)
==2211908==    by 0x1805EA: del_tree (tal.c:421)
==2211908==    by 0x1808BE: tal_free (tal.c:486)
==2211908==    by 0x12AB25: destroy_dc_from_conn (daemon_conn.c:112)
==2211908==    by 0x17FFDF: notify (tal.c:237)
==2211908==    by 0x180519: del_tree (tal.c:402)
==2211908==    by 0x1808BE: tal_free (tal.c:486)
==2211908==    by 0x16EE9A: io_close (io.c:450)
==2211908==    by 0x16ECA9: do_plan (io.c:401)
==2211908==    by 0x16ED16: io_ready (io.c:417)
==2211908==    by 0x1710B2: io_loop (poll.c:453)
==2211908==    by 0x1163C5: main (gossipd.c:1144)
==2211908==  Block was alloc'd at
==2211908==    at 0x484384F: malloc (vg_replace_malloc.c:381)
==2211908==    by 0x180064: allocate (tal.c:250)
==2211908==    by 0x180634: tal_alloc_ (tal.c:428)
==2211908==    by 0x12AB65: daemon_conn_new_ (daemon_conn.c:122)
==2211908==    by 0x1155F4: gossip_init (gossipd.c:763)
==2211908==    by 0x116014: recv_req (gossipd.c:999)
==2211908==    by 0x12A828: handle_read (daemon_conn.c:31)
==2211908==    by 0x16E09F: next_plan (io.c:59)
==2211908==    by 0x16ECD4: do_plan (io.c:407)
==2211908==    by 0x16ED16: io_ready (io.c:417)
==2211908==    by 0x1710B2: io_loop (poll.c:453)
==2211908==    by 0x1163C5: main (gossipd.c:1144)
==2211908== 
```
2022-03-23 13:20:12 +10:30
Rusty Russell 07c4d39b75 memleak: fix double-free if we timeout.
... and then dualopend returns, and we access the fread leak_detect struct.

```
lightningd: FATAL SIGNAL 6 (version 065ca1e)
0x55ecd4be8145 send_backtrace
	common/daemon.c:33
0x55ecd4be81f1 crashdump
	common/daemon.c:46
0x7f200acab51f ???
	./signal/../sysdeps/unix/sysv/linux/x86_64/libc_sigaction.c:0
0x7f200acff828 __pthread_kill_implementation
	./nptl/pthread_kill.c:44
0x7f200acff828 __pthread_kill_internal
	./nptl/pthread_kill.c:80
0x7f200acff828 __GI___pthread_kill
	./nptl/pthread_kill.c:91
0x7f200acab475 __GI_raise
	../sysdeps/posix/raise.c:26
0x7f200ac917b6 __GI_abort
	./stdlib/abort.c:79
0x55ecd4c6827f call_error
	ccan/ccan/tal/tal.c:93
0x55ecd4c68470 check_bounds
	ccan/ccan/tal/tal.c:165
0x55ecd4c684c2 to_tal_hdr
	ccan/ccan/tal/tal.c:175
0x55ecd4c68eb8 tal_free
	ccan/ccan/tal/tal.c:479
0x55ecd4b8bdd0 finish_report
	lightningd/memdump.c:138
0x55ecd4b8c115 leak_detect_req_done
	lightningd/memdump.c:201
0x55ecd4c68664 notify
	ccan/ccan/tal/tal.c:237
0x55ecd4c68b9e del_tree
	ccan/ccan/tal/tal.c:402
0x55ecd4c68bf3 del_tree
	ccan/ccan/tal/tal.c:412
0x55ecd4c68bf3 del_tree
	ccan/ccan/tal/tal.c:412
0x55ecd4c68f43 tal_free
	ccan/ccan/tal/tal.c:486
0x55ecd4c5751f io_close
	ccan/ccan/io/io.c:450
0x55ecd4bbce68 subd_shutdown_remaining
	lightningd/subd.c:911
0x55ecd4b8724a shutdown_subdaemons
	lightningd/lightningd.c:541
0x55ecd4b883cc main
	lightningd/lightningd.c:1207
0x7f200ac92fcf __libc_start_call_main
	../sysdeps/nptl/libc_start_call_main.h:58
0x7f200ac9307c __libc_start_main_impl
	../csu/libc-start.c:409
0x55ecd4b5cc54 ???
```

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2022-03-23 13:20:12 +10:30
Rusty Russell 486b1b2481 pytest: fix flake in test_funding_fail
We may not see a disconnect instantly:

```
>       assert len(l2.rpc.listpeers()['peers']) == 0
E       assert 1 == 0
E         +1
E         -0
```

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2022-03-23 13:20:12 +10:30
Rusty Russell aeeaa14430 pytest: add reconnect and restart to test_multichan.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2022-03-23 13:20:12 +10:30
Rusty Russell 293cf3c2b2 connect: delay return until all subds ready.
We had some flakes because we returned from `connect`, but we hadn't started
subds yet.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2022-03-23 13:20:12 +10:30
Rusty Russell 1e4149f18a lightningd: clean up peer connection handling a little.
Update the address and direction as soon as it connects not just when
we're about to make it active: we want this even if we don't have
an active channel, or if the connect hook rejects it.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2022-03-23 13:20:12 +10:30
Rusty Russell 8458994a7f pytest: test closing one multichannel works as expected.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2022-03-23 13:20:12 +10:30
Rusty Russell 4e8239fcfe lightningd: don't tell connectd to discard peer unless no subds left.
Otherwise it waits for subds to exit, but they don't.  Plus, the others
may still be talking!

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2022-03-23 13:20:12 +10:30
Rusty Russell 75596b3e0f lightningd: use a better channel if available to next hop.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2022-03-23 13:20:12 +10:30
Rusty Russell debc1b90d3 lightningd: remove checks which prevent us from opening multiple channels.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Added: Protocol: we now support opening multiple channels with the same peer.
2022-03-23 13:20:12 +10:30
Rusty Russell 21e1d68e3b lightningd: remove (most) functions to search channels by status.
This is generally verboten now, since there can be multiple.  There are a
few exceptions:

1. We sometimes want to know if there are *any* active channels.
2. Some dev commands still take peer id when they mean channel_id.
3. We still allow peer id when it's fully determined.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Changed: JSON-RPC: `close` by peer id will fail if there is more than one live channel (use `channel_id` or `short_channel_id` as id arg).
2022-03-23 13:20:12 +10:30
Rusty Russell f85425d106 lightningd: don't assume a single channel per peer.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2022-03-23 13:20:12 +10:30
Rusty Russell ba1242af3e lightningd: add find_channel_by_scid
More efficient to search a known peer than the whole set.

Also, move find_channel_by_id() from channel_control.c into channel.c
where we'd expect it.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2022-03-23 13:20:12 +10:30
Rusty Russell cb5dc48cab lightningd: make setchannelfee handle multiple channels per peer.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2022-03-23 13:20:12 +10:30
Rusty Russell b3438e9bba lightningd: associate connect commands with peer, not channel.
Sure, we want to connect (usually) because of an active channel, but
it's not specific to the channel itself.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2022-03-23 13:20:12 +10:30
Rusty Russell 33abf93ec1 lightningd: rename activate_peers() to setup_peers().
Activate means a specific thing now (connectd said something), so avoid
confusing it with this function.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2022-03-23 13:20:12 +10:30
Rusty Russell 90be2cc104 lightningd: remove some "single active channel" assumptions.
Generally this means converting a lazy "peer_active_channel(peer)" call
into an explicit iteration.

1. notify_feerate_change: call all channels (ignores non-active ones anyway).
2. peer_get_owning_subd remove unused function.
3. peer_connected hook: don't save channel, do lookup and iterate channels.
4. In json_setchannelfee "all" remove useless call to peer_active_channel
   since we check state anyway, and iterate.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2022-03-23 13:20:12 +10:30
Rusty Russell 7de7b7be61 lightningd: use channel_id when a peer is activated.
Rather than intuiting whether this is a new channel / active channel,
use the channel_id.  This simplifies things and makes them explicit,
and prepares for multiple live channels per peer.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2022-03-23 13:20:12 +10:30
Rusty Russell 32cd7ae398 connectd: key multiple subds by channel_id, use for lookup.
We still don't *have* multiple subds per peer, but now we could!

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2022-03-23 13:20:12 +10:30
Rusty Russell 395051cdf8 connectd: track the channel_id of each stream to/from peer.
This means doing some wire interpretation, and handling the transient
case where we switch from temporary to permenant channel_id, but it's
not that bad (and required for accurate demux when multiple channels
are involved for a single peer).

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2022-03-23 13:20:12 +10:30
Rusty Russell fe9f391a93 connectd: tell lightningd the channel_id when we give it the active peer.
Now we always have it (either extracted from an unsolicited message,
or told to us by lightningd when it tells us it wants to talk), we can
always send it.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2022-03-23 13:20:12 +10:30
Rusty Russell 2bc58e2327 lightningd: always tell connectd the channel id.
This means lightningd needs to create the temporary one and tell it to
openingd/dualopend, rather than the other way around.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2022-03-23 13:20:12 +10:30
Rusty Russell 57263a3eb2 lightningd: handle reestablish directly from connectd.
We don't need to hand it to channeld: it will read it!  We simply
need to tell it to expect it.

Similarly, openingd/dualopend will never see it, so remove that logic.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2022-03-23 13:20:12 +10:30
Rusty Russell 2424b7dea8 connectd: hold peer until we're interested.
Either because lightningd tells us it wants to talk, or because the peer
says something about a channel.

We also introduce a behavior change: we disconnect after a failed open.
We might want to modify this later, but we it's a side-effect of openingd
not holding onto idle connections.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2022-03-23 13:20:12 +10:30
Rusty Russell 77b1087cdf lightningd: move notification of disconnect into when we hear from connectd.
Simpler, and closes a potential race.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2022-03-23 13:20:12 +10:30
Rusty Russell deecedb033 connectd: tell lightningd when disconnect is complete.
This avoids races in our tests where we assume it's sync (and is kind
of nicer).

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2022-03-23 13:20:12 +10:30
Rusty Russell 6cc9f37cab connectd: handle connect vs closing race better.
We would return success from connect even though the peer was closing;
this is technically correct but fairly undesirable.  Better is to pass
every connect attempt to connectd, and have it block if the peer is
exiting (and retry), otherwise tell us it's already connected.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2022-03-23 13:20:12 +10:30
Rusty Russell 10e36e073c openingd: disconnect from peer when an error occurs.
openingd currently holds the connection to idle peers, but we're about
to change that: it will only look after peers which are actively
opening a connection.  We can start this process by disconnecting
whenever we have a negotiation failure.

We could stay connected if we wanted to, but that would be up to
connectd to decide.  Right now it's easier if we disconnect from any
idle peer once it's been active.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2022-03-23 13:20:12 +10:30
Rusty Russell 16e9ba0361 connectd: fix confusing names.
The message from lightningd simply acknowleges that we are allowed to
discard the peer (because no subdaemons are talking to it anymore).
This difference becomes more stark once connectd holds on to idle
peers.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2022-03-23 13:20:12 +10:30
Rusty Russell 0cba062318 pytest: add test to check we notice remote disconnects.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2022-03-23 13:20:12 +10:30
Rusty Russell eb203bf71e lightningd: clean up connect code.
1. The notification should be called every time.
2. channel can never be NULL, since it's tested above.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Fixed: JSON-RPC: `connect` notification now called even if we already have a live channel.
2022-03-23 13:20:12 +10:30
Rusty Russell fcd0b2eb42 connectd: prepare for multiple subd connections.
We still always have 1, but the infrastructure is now in place.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2022-03-23 13:20:12 +10:30
Rusty Russell 005d69c463 connectd: clean up decrypted packet memory handling.
Use tmpctx, rather than freeing manually everywhere (proof: next patch
added a branch and forgot to free it!).

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2022-03-23 13:20:12 +10:30
Rusty Russell 9bbb32433e connectd: make sure we do IO logging on final_msg output.
This happens when we send a warning or lightningd tells us to send a
final message then close.  Normally io logging is done by the
subdaemon that creates it, but this is a special case.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2022-03-23 13:20:12 +10:30
Rusty Russell b99c04e605 lightningd: add explicit "connected" flag.
We currently intuit this by whether there's a subdaemon owning it.
But we're about to change the rules and allow connectd to hold idle
connections, so we need an explicit flag.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2022-03-23 13:20:12 +10:30
Rusty Russell 915a591873 spender: free up vars to avoid transient false leak reports.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2022-03-23 13:20:12 +10:30
Christian Decker fdd7c6b192 pyln: Remove two more occurences of non-None default args 2022-03-22 19:17:13 +10:30