Commit Graph

784 Commits

Author SHA1 Message Date
Rusty Russell e46b0ff886 lightningd: fix crash on rapid reconnect.
Happens occasionally when running
`tests/test_connection.py::test_mutual_reconnect_race` (which is too
flaky to add, without more fixes):


```
lightningd: lightningd/peer_control.c:1252: peer_active: Assertion `!channel->owner' failed.
lightningd: FATAL SIGNAL 6 (version v0.11.0.1-38-g4f167da)
0x5594a41f8f45 send_backtrace
	common/daemon.c:33
0x5594a41f8fef crashdump
	common/daemon.c:46
0x7f7cb585c08f ???
	/build/glibc-SzIz7B/glibc-2.31/signal/../sysdeps/unix/sysv/linux/x86_64/sigaction.c:0
0x7f7cb585c00b __GI_raise
	../sysdeps/unix/sysv/linux/raise.c:51
0x7f7cb583b858 __GI_abort
	/build/glibc-SzIz7B/glibc-2.31/stdlib/abort.c:79
0x7f7cb583b728 __assert_fail_base
	/build/glibc-SzIz7B/glibc-2.31/assert/assert.c:92
0x7f7cb584cfd5 __GI___assert_fail
	/build/glibc-SzIz7B/glibc-2.31/assert/assert.c:101
0x5594a41b45ca peer_active
	lightningd/peer_control.c:1252
0x5594a418794c connectd_msg
	lightningd/connect_control.c:457
0x5594a41cd457 sd_msg_read
	lightningd/subd.c:556
0x5594a41ccbe5 read_fds
	lightningd/subd.c:357
0x5594a4269fc2 next_plan
	ccan/ccan/io/io.c:59
0x5594a426abca do_plan
	ccan/ccan/io/io.c:407
0x5594a426ac0c io_ready
	ccan/ccan/io/io.c:417
0x5594a426ceff io_loop
	ccan/ccan/io/poll.c:453
0x5594a41930d9 io_loop_with_timers
	lightningd/io_loop_with_timers.c:22
0x5594a4199293 main
	lightningd/lightningd.c:1181
0x7f7cb583d082 __libc_start_main
	../csu/libc-start.c:308
0x5594a416e15d ???
	???:0
0xffffffffffffffff ???
	???:0
```
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2022-06-20 11:37:24 +09:30
Rusty Russell 151d009435 lightningd: remove over-zealous assert.
This was hit on my node.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2022-04-13 10:32:29 +09:30
Michael Schmoock 21fd3b22d4 config: adds htlc_minimum_msat htlc_maximum_msat announce_discovered_ip
This adds config and commandline options for htlc_min_msat, htlc_max_msat and
announce_discovered_ip. The default is 0msat for htlc_min_msat, unlimited for
htlc_max_msat and enabled for announce_discovered_ip.

The announce_discovered_ip gets the disable commandline switch --disable-ip-discovery

Changelog-added: Config options for htlc_min_msat, htlc_max_msat and announce_discovered_ip.
2022-04-05 05:45:04 +09:30
Rusty Russell 2526e804f7 doc: big BOLT update to incorporate warnings language.
We do this (send warnings) in almost all cases anyway, so mainly this
is a textual update, but there are some changes:

1. Send ERROR not WARNING if they send a malformed commitment secret.
2. Send WARNING not ERROR if they get the shutdown_scriptpubkey wrong (vs upfront)
3. Send WARNING not ERROR if they send a bad shutdown_scriptpubkey (e.g. p2pkh in future)
4. Rename some vars 'err' to 'warn' to make it clear we send a warning.

This means test_option_upfront_shutdown_script can be made reliable, too,
and it now warns and doesn't automatically close channel.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2022-04-02 09:40:18 +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 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 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 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 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 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
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
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
niftynei 590f12145b listpeers: include the `pushed_msat` amount
Useful for accounting for missed/historical channel opens, to figure out
what the actual sat contribution from each peer is at a utxo level

Changelog-Added: JSONRPC: `listpeers` now includes a `pushed_msat` value. For leased channels, is the total lease_fee.
2022-03-05 15:12:12 +10:30
Simon Vrouwe 905a85dc99 JSON-RPC: getinfo, add field `our_features`
All build flags and (experimental) options make it hard to find
out what features are supported or enabled.

And the undocumented `--list-features-only`, does not account for all
our featurebits, for example bit 55 (keysend).

Changelog-Added: JSON-RPC: `getinfo` result now includes `our_features` (bits) for various Bolt #9 contexts
2022-03-01 09:38:40 +10:30
Rusty Russell b67329bec2 lightningd: don't leak address for printing.
Not really a leak, but it would last as long as the peer.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2022-02-27 07:44:26 +10:30
Michael Schmoock 38e2abf68a peer_exchange: set, read and log remote_addr
Changelog-Added: Protocol: set remote_addr on init tlvs
2022-02-22 05:45:47 +10:30
Rusty Russell d9b1e69243 dual_funding: don't steal inflight field in update_channel_from_inflight.
If we call update_channel_from_inflight *twice* with the same inflight, we
will get bad results.  Using tal_steal() here was a premature optimization:

```
Valgrind error file: valgrind-errors.496395
==496395== Invalid read of size 8
==496395==    at 0x22A9D3: to_tal_hdr (tal.c:174)
==496395==    by 0x22B4B5: tal_steal_ (tal.c:498)
==496395==    by 0x16A13D: update_channel_from_inflight (peer_control.c:1225)
==496395==    by 0x16A4C7: funding_depth_cb (peer_control.c:1299)
==496395==    by 0x182807: txw_fire (watch.c:232)
==496395==    by 0x182AA9: watch_topology_changed (watch.c:300)
==496395==    by 0x1290ED: updates_complete (chaintopology.c:624)
==496395==    by 0x129BF4: get_new_block (chaintopology.c:835)
==496395==    by 0x125EEF: getrawblockbyheight_callback (bitcoind.c:362)
==496395==    by 0x176ECC: plugin_response_handle (plugin.c:584)
==496395==    by 0x1770F5: plugin_read_json_one (plugin.c:690)
==496395==    by 0x1772D9: plugin_read_json (plugin.c:735)
==496395==  Address 0x89fbb08 is 24 bytes inside a block of size 104 free'd
==496395==    at 0x483CA3F: free (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==496395==    by 0x22B193: del_tree (tal.c:421)
==496395==    by 0x22B461: tal_free (tal.c:486)
==496395==    by 0x16A123: update_channel_from_inflight (peer_control.c:1223)
==496395==    by 0x16A4C7: funding_depth_cb (peer_control.c:1299)
==496395==    by 0x182807: txw_fire (watch.c:232)
==496395==    by 0x182AA9: watch_topology_changed (watch.c:300)
==496395==    by 0x1290ED: updates_complete (chaintopology.c:624)
==496395==    by 0x129BF4: get_new_block (chaintopology.c:835)
==496395==    by 0x125EEF: getrawblockbyheight_callback (bitcoind.c:362)
==496395==    by 0x176ECC: plugin_response_handle (plugin.c:584)
==496395==    by 0x1770F5: plugin_read_json_one (plugin.c:690)
==496395==  Block was alloc'd at
==496395==    at 0x483B7F3: malloc (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==496395==    by 0x22AC1C: allocate (tal.c:250)
==496395==    by 0x22B1DD: tal_alloc_ (tal.c:428)
==496395==    by 0x22B3A6: tal_alloc_arr_ (tal.c:471)
==496395==    by 0x22C094: tal_dup_ (tal.c:805)
==496395==    by 0x12B274: new_inflight (channel.c:187)
==496395==    by 0x136D4C: wallet_commit_channel (dual_open_control.c:1260)
==496395==    by 0x13B084: handle_commit_received (dual_open_control.c:2839)
==496395==    by 0x13B6AF: dual_opend_msg (dual_open_control.c:2976)
==496395==    by 0x1809FF: sd_msg_read (subd.c:553)
==496395==    by 0x218F5D: next_plan (io.c:59)
==496395==    by 0x219B65: do_plan (io.c:407)
```

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2022-02-08 11:15:52 +10:30
Rusty Russell 727b486d49 connectd: don't received useless peer fd if we're told to send final msg.
We don't need the connection to ourselves, just to free it.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2022-02-08 11:15:52 +10:30
Rusty Russell 3c5d27e3e9 subdaemons: remove gossipd fd from per-peer daemons.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2022-02-08 11:15:52 +10:30
Rusty Russell 1c71c9849b connectd: handle custom messages.
This is neater than what we had before, and slightly more general.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Changed: JSON_RPC: `sendcustommsg` now works with any connected peer, even when shutting down a channel.
2022-02-08 11:15:52 +10:30
niftynei 081ef05dd1 listchannels: add funding_outnum to listchannels
Changelog-Added: JSONRPC: `listchannels` now includes the `funding_outnum`
2022-02-07 13:02:09 +10:30
Rusty Russell 39c93ee6e5 connectd: get addresses from lightningd, not gossipd.
It's weird to have connectd ask gossipd, when lightningd can just do it
and hand all the addresses together.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2022-01-20 15:24:06 +10:30
Rusty Russell 741f44725a patch lightningd-peer-fds.patch 2022-01-20 15:24:06 +10:30
Rusty Russell 6115ed02e8 subdaemons: don't stream gossip_store at all.
We now let gossipd do it.

This also means there's nothing left in 'struct per_peer_state' to
send across the wire (the fds are sent separately), so that gets
removed from wire messages too.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2022-01-20 15:24:06 +10:30
Rusty Russell 71f736678f lightningd: make sure gossipd knows before we update blockheight.
Without this, we can get spurious failures from lnprototest, which
is waiting for lightningd to acknowledge the blockheight in getinfo:

```
2021-12-21T00:56:21.460Z DEBUG   lightningd: Adding block 109: 57a7bd3ade3a88a899e5b442075e9722ccec07e0205f9a913b304a3e2e038e26
2021-12-21T00:56:21.470Z DEBUG   lightningd: Adding block 110: 11a280eb76f588e92e20c39999be9d2baff016c3c6bac1837b649a270570b7dd
2021-12-21T00:56:21.479Z DEBUG   lightningd: Adding block 111: 02977fc9529b2ab4e0a805c4bc1bcfbff5a4e6577a8b31266341d22e204a1d27
2021-12-21T00:56:21.487Z DEBUG   lightningd: Adding block 112: 2402f31c5ddfc9e847e8bbfb7df084d29a5d5d936a4358c109f2f4cf9ea8d828
2021-12-21T00:56:21.496Z DEBUG   lightningd: Adding block 113: 5a561fe9423b4df33f004fc09985ee3ef38364d692a56a8b27ecbc6098a16d39
2021-12-21T00:56:21.505Z DEBUG   lightningd: Adding block 114: 4502f5ec23c89177872846848848322e8fa6c3fb6f5eb361194e4cd47596dfe9
2021-12-21T00:56:21.511Z DEBUG   02f9308a019258c31049344f85f89d5229b531c845836f99b08601f113bce036f9-gossipd: Ignoring future channel_announcment for 109x1x0 (current block 108)
```

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2022-01-20 15:24:06 +10:30