Commit Graph

96 Commits

Author SHA1 Message Date
Rusty Russell a5fee67b91 common/memleak: take over dump_memleak(), allow print pointer.
This will let plugins use it.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2021-09-08 19:11:47 +02:00
Michael Schmoock 24ea498350 cleanup: rename use_proxy_always to always_use_proxy to match cfg
This renames all occurences of use_proxy_always to always_use_proxy
to keep it inline with config values. This was a bit confusing.

Only significant change is that the payload in the plugins init
requests also contained the old name. No plugin currently seems to make
use of this variable yet. The old name 'use_proxy_always' is added when
deprecated APIs is enabled.

Changelog-Deprecated: Plugins: Renames plugin init 'use_proxy_always' to 'always_use_proxy'
2021-08-23 14:43:40 +09:30
Michael Schmoock 0550ffa9db cleanup: nits 2021-08-23 14:43:40 +09:30
Michael Schmoock f31c5b7662 connectd: prefer non-TOR connections and filter wireaddr hint duplicate
This does two things:
 - It moves non-tor addresses upfront so it prefers normal connection
   which are less laggy and more reliable.
 - It prevents connectd from trying the same wire_addr twice when the
   addr_hint was given and gossip also added the same address.

Changelog-Changed: connectd: try non-TOR connections first
Changelog-Fixed: connectd: do not try address hint twice
2021-08-23 14:43:40 +09:30
Rusty Russell 9eb531868f lightningd: make invoices insist on payment_secret.
In fact, we make it compulsory, which means if you don't understand it
you'll hang up on us!

Add some logging for that in future.

Changelog-Changed: Protocol: All new invoices require a payment_secret (i.e. modern TLV format onion)
Changelog-Changed: Protocol: We can no longer connect to peers which don't support `payment_secret`.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2021-07-14 14:38:00 -05:00
Rusty Russell 4cc3eaf79c common/bech32: update to bech32m reference.
I did this by copying the updated bech32 code, and then re-patching in
our minor changes:

1. Headers modded (we need size_t)
2. Explicit length for bech32_encode/decode (not 90).
3. Exposing and bech32_ prefix for convert_bits, charset, charset_rev.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2021-06-11 13:54:20 +09:30
Rusty Russell e2f225e4cd lightningd: use connectd to send the final error message, not openingd/dualopend
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2021-06-03 16:16:34 +09:30
Rusty Russell 7cffea10ce connectd: new command to make us send message and close.
Currently we abuse openingd and dualopend to do this, but connectd already
has the ability to talk to peers, so it's more efficient.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2021-06-03 16:16:34 +09:30
Rusty Russell e960b1015d !fixup
Log this so I can try to find dual fund connnectd race.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2021-05-11 11:25:16 +09:30
Rusty Russell b0d6996ed6 lightningd: get connection direction from connectd.
This matters: if we connected, the address is probably usable for future connections.
But if they connected, the port is probably not (but the IP address may be).

Changelog-Added: JSON-RPC: `connect` returns "direction" ("in": they iniatated, or "out": we initiated)
Changelog-Added: plugins: `peer_connected` hook and `connect` notifications have "direction" field.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2021-03-26 13:22:33 +10:30
Rusty Russell 6dae0118f9 connectd: clearly differentiate incoming and outgoing paths.
This should make it clearer where the problem seen in
https://github.com/ElementsProject/lightning/issues/4297 is.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2021-03-25 11:31:58 +10:30
Jan Sarenik 1b02d15695 typo: information is an uncountable mass noun
See https://en.wikipedia.org/wiki/Information

In libplugin.c also the word "details" was added (without removing
the 'information').

Changelog-None
2021-03-16 10:45:40 +10:30
Rusty Russell 6b11cc8b8c common: disallow NULL channel_id to peer_failed_err.
No more sending "all-channel" errors; in particular, gossipd now only
sends warnings (which make us hang up), not errors, and peer_connected
rejections are warnings (and disconnect), not errors.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Changed: Plugins: `peer_connected` rejections now send a warning, not an error, to the peer.
2021-02-04 12:02:52 +10:30
Rusty Russell 82ed71d621 connectd: don't crash if connect() fails immediately.
Took me a while (stressing under valgrind) to reproduce this,
then longer to figure out how it happened.

Turns out io_new_conn() can fail if the init function fails.
In our case, this can happen if connect() immediately returns
an error (inside io_connect).  But we've already set the finish
function, which (if this was the last address), will free connect,
making the assignment `connect->conn = ...` write to a freed address.

Either way, if it fails, try_connect_one_addr() has taken care to
update connect->conn, or free connect, and the caller should not do it.

Here's the valgrind trace:
```
==384981== Invalid write of size 8
==384981==    at 0x11127C: try_connect_one_addr (connectd.c:880)
==384981==    by 0x112BA1: destroy_io_conn (connectd.c:708)
==384981==    by 0x141459: destroy_conn (poll.c:244)
==384981==    by 0x14147F: destroy_conn_close_fd (poll.c:250)
==384981==    by 0x149EB9: notify (tal.c:240)
==384981==    by 0x149F8B: del_tree (tal.c:402)
==384981==    by 0x14A51A: tal_free (tal.c:486)
==384981==    by 0x140036: io_close (io.c:450)
==384981==    by 0x1400B3: do_plan (io.c:401)
==384981==    by 0x140134: io_ready (io.c:423)
==384981==    by 0x141A57: io_loop (poll.c:445)
==384981==    by 0x112CB0: main (connectd.c:1703)
==384981==  Address 0x4d67020 is 64 bytes inside a block of size 160 free'd
==384981==    at 0x483CA3F: free (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==384981==    by 0x14A020: del_tree (tal.c:421)
==384981==    by 0x14A51A: tal_free (tal.c:486)
==384981==    by 0x1110C5: try_connect_one_addr (connectd.c:806)
==384981==    by 0x112BA1: destroy_io_conn (connectd.c:708)
==384981==    by 0x141459: destroy_conn (poll.c:244)
==384981==    by 0x14147F: destroy_conn_close_fd (poll.c:250)
==384981==    by 0x149EB9: notify (tal.c:240)
==384981==    by 0x149F8B: del_tree (tal.c:402)
==384981==    by 0x14A51A: tal_free (tal.c:486)
==384981==    by 0x140036: io_close (io.c:450)
==384981==    by 0x1405DC: io_connect_ (io.c:345)
==384981==  Block was alloc'd at
==384981==    at 0x483B7F3: malloc (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==384981==    by 0x149CF1: allocate (tal.c:250)
==384981==    by 0x14A3C6: tal_alloc_ (tal.c:428)
==384981==    by 0x1114F2: try_connect_peer (connectd.c:1526)
==384981==    by 0x111717: connect_to_peer (connectd.c:1558)
==384981==    by 0x1124F5: recv_req (connectd.c:1627)
==384981==    by 0x1188B2: handle_read (daemon_conn.c:31)
==384981==    by 0x13FBCB: next_plan (io.c:59)
==384981==    by 0x140076: do_plan (io.c:407)
==384981==    by 0x140113: io_ready (io.c:417)
==384981==    by 0x141A57: io_loop (poll.c:445)
==384981==    by 0x112CB0: main (connectd.c:1703)
```

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Fixed: Occasional crash in connectd due to use-after-free
Fixes: #4343
2021-02-01 21:01:06 +01:00
niftynei abad494fcf connectd: properly cleanup 'competing' outgoing connections
Coauthored-By: Rusty Russell @rustyrussell
2020-11-16 20:00:51 -06:00
niftynei 4a3ee19a22 connectd: Update connection list with new address
If we're already attempting to connect to a peer, we would ignore
new connection requests. This is problematic if your node has bad
connection details for the node -- you can't update it while inflight.

This patch appends new connection suggestions to the list of connections
to try.

Fixes #4154
2020-11-16 20:00:51 -06:00
Rusty Russell f37f2b6193 common/memleak: simplify and document API.
1. Rename memleak_enter_allocations to memleak_find_allocations.
2. Unify scanning for pointers into memleak_remove_region / memleak_remove_pointer.
3. Document the functions.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2020-09-23 13:52:49 +09:30
Rusty Russell f658dd0d78 pytest: test connection timeout.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2020-09-11 21:27:45 +09:30
Rusty Russell 7f2b332021 connectd: implement connection timeout (60 seconds).
This is simple, and we now can multifundchannel to every node on testnet
(one simply hangs once we connect).

Changelog-Fixed: Protocol: We now hang up if peer doesn't respond to init message after 60 seconds.
2020-09-11 21:27:45 +09:30
Rusty Russell c34c055d82 Makefile: use completely separate spec-derived files for EXPERIMENTAL_FEATURES
This avoids overwriting the ones in git, and generally makes things neater.

We have convenience headers wire/peer_wire.h and wire/onion_wire.h to
avoid most #ifdefs: simply include those.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2020-09-08 09:42:00 +09:30
Rusty Russell 8150d28575 Makefile: use generic rules to make spec-derived sources.
Now we use the same Makefile rules for all CSV->C generation.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2020-08-31 21:33:26 -05:00
Rusty Russell 398b4806b9 connectd: convert to new wire generation style.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2020-08-25 12:53:13 +09:30
Rusty Russell dffbf8de85 gossipd: convert wire to new scheme.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2020-08-25 12:53:13 +09:30
Rusty Russell 1702c7a69a hsmd: convert to new wire generation style.
Note that other directories were explicitly depending on the generated
file, instead of relying on their (already existing) dependency on 
$(LIGHTNINGD_HSM_CLIENT_OBJS), so we remove that.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2020-08-25 12:53:13 +09:30
Rusty Russell 168009c105 features: require dependent features at init handshake.
This simplifies our test matrix, as we never have to handle talking
to peers that specify one but not the other.

This is particularly important for option_anchor_outputs which
assumes option_static_remotekey.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2020-08-14 11:51:14 +09:30
Saibato 2b0aba13a8 connectd/connectd: Display an error hint for V3 tor onions when connect fail.
@thestick613 noticed that since tor version below 0.3.2.2-alpha
will not support V3 ed25519 address formats, the error handling
is not that helpful in the error message from cli.
So now we add an hint.

Changelog-None:

Signed-off-by: Saibato <saibato.naga@pm.me>

connectd/connectd.h; Add helper function to update conn error list

Signed-off-by: Saibato <saibato.naga@pm.me>
2020-07-01 11:21:58 +02:00
Sebastian Falbesoner 84ef0cf7fa connectd: fix '~' comment (s/pubkey/node_id/)
Usage of node_id was introduced with commit a2fa699e0e.

Changelog-None
2020-05-04 12:32:02 +02:00
darosior 7ae8e21247 connectd: add darosior's seed node 2020-04-14 10:09:38 +09:30
Rusty Russell 3b4a06f52b common: generalize ecdh function.
common/onion is going to need to use this for the case where it finds a blinding
seed inside the TLV.  But how it does ecdh is daemon-specific.

We already had this problem for devtools/gossipwith, which supplied a
special hsm_do_ecdh().  This just makes it more general.

So we create a generic ecdh() interface, with a specific implementation
which subdaemons and lightningd can use.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2020-04-04 16:08:49 +10:30
Rusty Russell 2f1502abf4 cleanup: make 'u8 *features' and 'struct feature_set *fset' more explicit.
It's almost always "their_features" and "our_features" respectively, so
make those names clear.

Suggested-by: @cdecker
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2020-04-03 13:13:21 +10:30
Rusty Russell cf43e44378 common/features: don't use internal global.
Turns out that unnecessary: all callers can access the feature_set,
so make it much more like a normal primitive.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2020-04-03 13:13:21 +10:30
Rusty Russell 15f54878e4 connectd: do feature bits check after init exchange.
This will help with the next patch, where we wean off using a global
for features: connectd.c has access to the feature bits.

Since connectd might now want to send a message, it needs the crypto_state
non-const, which makes this less trivial than it would otherwise be.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2020-04-03 13:13:21 +10:30
Rusty Russell b7db588a8a plugin: remove boutique features.
We now manipulate the global features directly.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2020-03-31 13:36:02 +02:00
Rusty Russell c95e58ad4b subdaemons: initialize feature routines with explicit feature_set.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2020-03-31 13:36:02 +02:00
Rusty Russell a430abf899 connectd: permit multiple descriptors of the same type.
This restriction was removed from the spec as of
86c2ebcc5973a4133d3ce4d80ae1c203061a1646.

We also fix up some strange formatting in that part of the documentation.

Changelog-changed: We now announce multiple addresses of the same type, if given.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2020-03-31 13:36:02 +02:00
Rusty Russell 2aad3ffcf8 common: tal_dup_talarr() helper.
This is a common thing to do, so create a macro.

Unfortunately, it still needs the type arg, because the paramter may
be const, and the return cannot be, and C doesn't have a general
"(-const)" cast.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2020-02-27 14:16:16 +10:30
Christian Decker 8d6c8c3cd1 connectd: Pass the init_featurebits down to connectd and use in init
The `init_featurebits` are computed at startup, and then cached
indefinitely. They are then used whenever a new `init` handshake is performed.

We could add a new message to push updates to `connectd` whenever a plugin is
added or removed, but that's up for discussion.
2020-02-11 13:53:31 +10:30
Vasil Dimov 55173a56b7 Use dedicated type for error codes
Before this patch we used `int` for error codes. The problem with
`int` is that we try to pass it to/from wire and the size of `int` is
not defined by the standard. So a sender with 4-byte `int` would write
4 bytes to the wire and a receiver with 2-byte `int` (for example) would
read just 2 bytes from the wire.

To resolve this:

* Introduce an error code type with a known size:
  `typedef s32 errcode_t`.

* Change all error code macros to constants of type `errcode_t`.
  Constants also play better with gdb - it would visualize the name of
  the constant instead of the numeric value.

* Change all functions that take error codes to take the new type
  `errcode_t` instead of `int`.

* Introduce towire / fromwire functions to send / receive the newly added
  type `errcode_t` and use it instead of `towire_int()`.

In addition:

* Remove the now unneeded `towire_int()`.

* Replace a hardcoded error code `-2` with a new constant
  `INVOICE_EXPIRED_DURING_WAIT` (903).

Changelog-Changed: The waitinvoice command would now return error code 903 to designate that the invoice expired during wait, instead of the previous -2
2020-01-31 06:02:47 +00:00
Vasil Dimov fb7c006187 wire: add towire_int() and use it in connectd
Add towire_int() and fromwire_int() functions to "(de)serialize"
"int". This will only work as long as both the caller of towire_int()
and the caller of fromwire_int() use the same in-memory representation
of signed integers and have the same sizeof(int).

Changelog-None
2020-01-21 16:59:18 +01:00
Vasil Dimov fc75d8a9e6 connectd: add own err codes instead of generic -1
Make it possible for connectd to send an error code to lightningd in
addition to the error message. Introduce two new error codes, replacing
the catch-all -1.

This change, together with
https://github.com/ElementsProject/lightning/pull/3395
will implement https://github.com/ElementsProject/lightning/issues/3366

Changelog-Changed: The `connect` command now returns its own error codes instead of a generic -1.
2020-01-21 16:59:18 +01:00
lisa neigut f6ff5e5b19 connectd: make failure message more descriptive 2019-12-04 15:32:31 -06:00
Saibato f6006f43a9 Init commit to be able to create a tor static service on the fly.
We  want to have a static Tor service created from a blob bound to
our node on cmdline

Changelog-added: persistent Tor address support
Changelog-added: allow the Tor inbound service port differ from 9735

Signed-off-by: Saibato <saibato.naga@pm.me>

Add base64 encode/decode to common

We need this to encode the blob for the tor service

Signed-off-by: Saibato <saibato.naga@pm.me>
2019-12-03 23:35:18 +01:00
darosior e6b8a02446 connectd: setup chainparams
We are going to signal the genesis block hash in the init message.
2019-11-29 21:17:08 +01:00
Rusty Russell eed654f684 connectd, gossipd: use per-peer logging.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2019-11-18 04:50:22 +00:00
Saibato 187d2e0f26 We state not to do 'any local DNS' if --always-use-proxy flag is set, so we should this
Even if it is on startup only once ...
Like @bitcoin-software indicated the expected UX should be in
line with what a user expects the software will do
so we should not dns if we say so with a flag that suggest that.

Changelog-Fixed: We disable all dns even on startup the scan for bogus dns servers, if --always-use-proxy is set true

Signed-off-by: Saibato <saibato.naga@pm.me>
2019-11-11 00:04:07 +00:00
Rusty Russell fe17acf07b TAGS: reformat to fix when PRINTF_FMT() used.
I was wondering why TAGS was missing some functions, and finally
tracked it down: PRINTF_FMT() confuses etags if it's at the start
of a function, and it ignores the rest of the file.

So we put PRINTF_FMT at the end, but that doesn't work for
*definitions*, only *declarations*.  So we remove it from definitions
and add gratuitous declarations in the few static places.1

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2019-11-01 17:27:20 -05:00
Christian Decker 3c3d7e2df4 connectd: Do not clobber the for-variable when resolving over DNS
We were using `i` as index variable in two nested loops. This works as long as
the DNS seed resolves to a single address, but will crash if the node has both
an A as well as an AAAA entry, at which point we'll try to index the hostname
without a matching entry.

Signed-off-by: Christian Decker <@cdecker>
2019-10-18 14:34:49 +02:00
Rusty Russell bd55f6d940
common/features: only support a single feature bitset.
This is mainly an internal-only change, especially since we don't
offer any globalfeatures.

However, LND (as of next release) will offer global features, and also
expect option_static_remotekey to be a *global* feature.  So we send
our (merged) feature bitset as both global and local in init, and fold
those bitsets together when we get an init msg.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2019-10-11 02:52:04 +00:00
darosior 274e32925f connectd: add a BOLT10 reference when doing DNS lookup 2019-09-30 01:18:22 +00:00
darosior a4204226b4 Tidy up parse_wireaddr_from_hostname 2019-09-28 02:49:24 +00:00