Commit Graph

3449 Commits

Author SHA1 Message Date
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 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 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 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 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 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 5704653d4c setchannel: don't let them advertize htlc_maximum_msat larger than capacity.
And check for the obvious setting min > max.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2022-03-22 18:45:41 +10:30
Rusty Russell 999c734bb5 setchannel: add minhtlc
Suggested by @m-schmook, I realized that if we append it later I'll
never get it right: I expect parameters min and max, not max and min!

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Added: Protocol: you can now alter the `htlc_minimum_msat` and `htlc_maximum_msat` your node advertizes.
2022-03-22 18:45:41 +10:30
Rusty Russell f29890ed66 lightningd: check htlc_maximum_msat of channels for routehints.
We still use the channel hint here (as it's the only option), we just
warn about lack of capacity.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2022-03-22 18:45:41 +10:30
Rusty Russell 1751b1becc pytest: add checks that pay and getroute respect htlc_maximum_msat.
We need to add some, since our internal representations of
htlc_maximum_msat round up, and we need to disable mpp which succeeds
in getting a payment through by splitting.

We also allow dev_routes to suppress invoice routehints altogether.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2022-03-22 18:45:41 +10:30
Rusty Russell 42f91ff2fa lightningd: deprecate setchannelfee, use setchannel in tests.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Deprecated: JSON-RPC: `setchannelfee` (use `setchannel`).
2022-03-22 18:45:41 +10:30
Rusty Russell 66e264d6b3 lightningd: new setchannel command.
Based on setchannelfee, but expanded to allow setting max htlc amount (and others
in future?).

The main differences:
1. It doesn't change values which are not specified (that would be hard to
   add fields to!)
2. It says exactly what all values are in any potentially changed channels.

Changelog-Added: JSON-RPC: new `setchannel` command generalizes `setchannelfee`.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2022-03-22 18:45:41 +10:30
Rusty Russell 4fdcee9a11 channeld: generalize specific_feerates mesage into config_channel.
1. Add the htlc_max param.
2. Allow parameters to be unset, meaning "don't change".

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2022-03-22 18:45:41 +10:30
Rusty Russell 33bd251296 listpeers: show maximum_htlc_out_msat.
This is htlc_maximum_msat in BOLT 7 speak, but this name matches our existing
fields and is clearer in this context.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2022-03-22 18:45:41 +10:30
Rusty Russell f078a916e7 lightningd: enforce htlc_maximum_msat.
When we let them set it, this matters!

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2022-03-22 18:45:41 +10:30
Rusty Russell 3217dbe17f channeld: get htlc_maximum_msat from lightningd.
We used to calculate it ourselves.  Unfortunately this needs to
be done in several places, since new_channel() isn't used to fully
create a channel in the case of dual funding :(

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2022-03-22 18:45:41 +10:30
Rusty Russell 6fdcc86f9d lightningd: store htlc_maximum_msat for channel in the db.
We currently don't allow setting it, but it's been requested.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2022-03-22 18:45:41 +10:30
Rusty Russell 65be18d355 memleak: handle libwally allocations better.
Things allocated by libwally all get the tal_name "wally_tal",
which cost me a few hours trying to find a leak.

In the case where we're making one of the allocations the parent
of the others (e.g. a wally_psbt), we can do better: supply a name
for the tal_wally_end().

So I add a new tal_wally_end_onto() which does the standard
tal_steal() trick, and also changes the (typechecked!) name.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2022-03-21 10:19:50 +10:30
Rusty Russell 43a833e405 lightningd: remove support for legacy onion format.
As per proposal in https://github.com/lightning/bolts/pull/962

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Removed: protocol: support for legacy onion format removed, since everyone supports the new one.
2022-03-18 09:20:11 +10:30
Rusty Russell 0db05f6e9c lightningd: opt_var_onion is now a compulsory feature.
We're about to drop support for legacy.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2022-03-18 09:20:11 +10:30
Ken Sedgwick 36466af3eb hsmd: Add fields to hsmd_sign_{,remote_}commitment_tx for validating signers 2022-03-16 12:06:21 +10:30
Michael Schmoock db95893aec lightningd: do not use remote_addr for always_use_proxy 2022-03-11 16:42:45 +10:30
Michael Schmoock 67fdc6f8ad gossipd: send updated node_annoucement remote_addr
This is the cheapest algo I came up with that simply checks that the
same `remote_addr` has been report by two different peers. Can be
improved in many ways:

 - Check by connecting to a radonm peers in the network
 - Check for more than two confirmations or a certain fraction
 - ...

Changelog-Added: Send updated node_annoucement when two peers report the same remote_addr.
2022-03-11 16:42:45 +10:30
Michael Schmoock 28b4e57974 lightningd: store recently reported remote_addr 2022-03-11 16:42:45 +10:30
Michael Schmoock e92176248e chore: fix typo announcable -> announceable
"announcable" is a common misspelling of "announceable", see:

https://en.wiktionary.org/wiki/announcable
2022-03-11 16:42:45 +10:30
Ken Sedgwick 8f56f96801 hsmd: Add wallet index metadata to existing messages 2022-03-10 10:22:29 +10:30
Ken Sedgwick 3abe22213c hsmd: Augment call to hsmd_sign_withdrawal_tx with wallet index metadata 2022-03-10 10:22:29 +10:30
Rusty Russell d7ffb712e5 dualopend: restore memleak calls.
And implement a timeout (20 seconds) just in case it's not listening.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2022-03-10 09:40:09 +10:30
Rusty Russell 00bb6f07d7 lightningd: simplify memleak code.
Instead of doing this weird chaining, just call them all at once and
use a reference counter.

To make it simpler, we return the subd_req so we can hang a destructor
off it which decrements after the request is complete.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2022-03-10 09:40:09 +10:30
Rusty Russell b5a1715c2b connectd: also fail without a scary backtrace when listen fails.
For example, if you do:

```
./lightningd/lightningd --network=regtest --experimental-websocket-port=19846
```

Then you're trying to reuse the normal port as the websocket port, but this
only fails at *listen* time, when we activate connectd.  Catch this too.

Fixes incorrect fatal() message, too.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2022-03-05 15:48:03 +10:30
Rusty Russell f1ed373c97 connectd: be more graceful when we an address is in use.
Aditya had this issue due to a config line, and the result was hard
to diagnose even for me.

It's now:

```
$ ./lightningd/lightningd --network=regtest --addr=:18444
2022-02-26T05:01:28.705Z **BROKEN** connectd: Failed to bind socket for 0.0.0.0:18444: Address already in use
```

Whereas before it doesn't even give the address it's trying to bind:

```
rusty@rusty-XPS-13-9370:~/devel/cvs/lightning (master)$ ./lightningd/lightningd --network=regtest --addr=:18444
lightning_connectd: Failed to bind on 2 socket: Address already in use (version v0.10.2-331-g86b83e4)
0x558a8b8d9a12 send_backtrace
	common/daemon.c:33
0x558a8b8e91e1 status_failed
	common/status.c:221
0x558a8b8c8e4f make_listen_fd
	connectd/connectd.c:1090
0x558a8b8c8f55 handle_wireaddr_listen
	connectd/connectd.c:1129
0x558a8b8c993d setup_listeners
	connectd/connectd.c:1312
0x558a8b8ca344 connect_init
	connectd/connectd.c:1517
0x558a8b8cbb57 recv_req
	connectd/connectd.c:1896
0x558a8b8d9f9f handle_read
	common/daemon_conn.c:31
0x558a8b9247c1 next_plan
	ccan/ccan/io/io.c:59
0x558a8b9253c9 do_plan
	ccan/ccan/io/io.c:407
0x558a8b92540b io_ready
	ccan/ccan/io/io.c:417
0x558a8b9276fe io_loop
	ccan/ccan/io/poll.c:453
0x558a8b8cbf36 main
	connectd/connectd.c:2033
0x7fe4d02940b2 ???
	???:0
0x558a8b8c285d ???
	???:0
0xffffffffffffffff ???
	???:0
2022-02-26T05:02:27.547Z **BROKEN** connectd: Failed to bind on 2 socket: Address already in use (version v0.10.2-331-g86b83e4)
2022-02-26T05:02:27.547Z **BROKEN** connectd: backtrace: common/daemon.c:38 (send_backtrace) 0x558a8b8d9a68
2022-02-26T05:02:27.547Z **BROKEN** connectd: backtrace: common/status.c:221 (status_failed) 0x558a8b8e91e1
2022-02-26T05:02:27.547Z **BROKEN** connectd: backtrace: connectd/connectd.c:1090 (make_listen_fd) 0x558a8b8c8e4f
2022-02-26T05:02:27.548Z **BROKEN** connectd: backtrace: connectd/connectd.c:1129 (handle_wireaddr_listen) 0x558a8b8c8f55
2022-02-26T05:02:27.548Z **BROKEN** connectd: backtrace: connectd/connectd.c:1312 (setup_listeners) 0x558a8b8c993d
2022-02-26T05:02:27.548Z **BROKEN** connectd: backtrace: connectd/connectd.c:1517 (connect_init) 0x558a8b8ca344
2022-02-26T05:02:27.548Z **BROKEN** connectd: backtrace: connectd/connectd.c:1896 (recv_req) 0x558a8b8cbb57
2022-02-26T05:02:27.548Z **BROKEN** connectd: backtrace: common/daemon_conn.c:31 (handle_read) 0x558a8b8d9f9f
2022-02-26T05:02:27.548Z **BROKEN** connectd: backtrace: ccan/ccan/io/io.c:59 (next_plan) 0x558a8b9247c1
2022-02-26T05:02:27.548Z **BROKEN** connectd: backtrace: ccan/ccan/io/io.c:407 (do_plan) 0x558a8b9253c9
2022-02-26T05:02:27.548Z **BROKEN** connectd: backtrace: ccan/ccan/io/io.c:417 (io_ready) 0x558a8b92540b
2022-02-26T05:02:27.548Z **BROKEN** connectd: backtrace: ccan/ccan/io/poll.c:453 (io_loop) 0x558a8b9276fe
2022-02-26T05:02:27.548Z **BROKEN** connectd: backtrace: connectd/connectd.c:2033 (main) 0x558a8b8cbf36
2022-02-26T05:02:27.548Z **BROKEN** connectd: backtrace: (null):0 ((null)) 0x7fe4d02940b2
2022-02-26T05:02:27.548Z **BROKEN** connectd: backtrace: (null):0 ((null)) 0x558a8b8c285d
2022-02-26T05:02:27.548Z **BROKEN** connectd: backtrace: (null):0 ((null)) 0xffffffffffffffff
2022-02-26T05:02:27.548Z **BROKEN** connectd: STATUS_FAIL_INTERNAL_ERROR: Failed to bind on 2 socket: Address already in use
lightningd: connectd failed (exit status 242), exiting.
```

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2022-03-05 15:48:03 +10:30