This adds a new hook: onion_message_ourpath for when we know a message
came in via a blinded path we created. The onion_message_blinded hook
is now called for all other messages, since all messages are now
blinded.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
When we are calling hooks, we track them via a linked list. As they
execute, we pop them off the list in plugin_hook_killed().
When we kill a plugin, we have a destructor which remove its entry from the linked list: plugin_hook_killed.
If it's at the head of the list, that means the plugin died while
processing the hook, so instead of just deleting it, we call
plugin_hook_killed() which behaves as if it said "result: continue".
But plugin_hook_killed() just returns if we're shutting down; this
leaves the link (then freed) on the list, and the *next* plugin tries
to unlink from the list, accessing the previous free entry.
The fix is simple: unlink from the list in plugin_hook_killed() even
if we're shutting down.
```
Valgrind error file: valgrind-errors.78570
==78570== Invalid write of size 8
==78570== at 0x174B55: list_del_ (list.h:328)
==78570== by 0x174FCC: plugin_hook_killed (plugin_hook.c:135)
==78570== by 0x21DC3F: notify (tal.c:240)
==78570== by 0x21E156: del_tree (tal.c:402)
==78570== by 0x21E1A8: del_tree (tal.c:412)
==78570== by 0x21E4F2: tal_free (tal.c:486)
==78570== by 0x16EBD1: plugin_kill (plugin.c:345)
==78570== by 0x16F9C4: plugin_conn_finish (plugin.c:724)
==78570== by 0x20F1A5: destroy_conn (poll.c:244)
==78570== by 0x20F1C9: destroy_conn_close_fd (poll.c:250)
==78570== by 0x21DC3F: notify (tal.c:240)
==78570== by 0x21E156: del_tree (tal.c:402)
==78570== Address 0x6aee688 is 40 bytes inside a block of size 72 free'd
==78570== at 0x483CA3F: free (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==78570== by 0x21E224: del_tree (tal.c:421)
==78570== by 0x21E1A8: del_tree (tal.c:412)
==78570== by 0x21E4F2: tal_free (tal.c:486)
==78570== by 0x16EBD1: plugin_kill (plugin.c:345)
==78570== by 0x16F9C4: plugin_conn_finish (plugin.c:724)
==78570== by 0x20F1A5: destroy_conn (poll.c:244)
==78570== by 0x20F1C9: destroy_conn_close_fd (poll.c:250)
==78570== by 0x21DC3F: notify (tal.c:240)
==78570== by 0x21E156: del_tree (tal.c:402)
==78570== by 0x21E4F2: tal_free (tal.c:486)
==78570== by 0x20D7B6: io_close (io.c:450)
==78570== Block was alloc'd at
==78570== at 0x483B7F3: malloc (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==78570== by 0x21DCAD: allocate (tal.c:250)
==78570== by 0x21E26E: tal_alloc_ (tal.c:428)
==78570== by 0x175599: plugin_hook_call_ (plugin_hook.c:259)
==78570== by 0x13616F: plugin_hook_call_onion_message_blinded (onion_message.c:126)
==78570== by 0x13643B: handle_obs_onionmsg_to_us (onion_message.c:187)
==78570== by 0x138BBD: gossip_msg (gossip_control.c:140)
==78570== by 0x178AEC: sd_msg_read (subd.c:495)
==78570== by 0x20CA00: next_plan (io.c:59)
==78570== by 0x20D608: do_plan (io.c:407)
==78570== by 0x20D64A: io_ready (io.c:417)
==78570== by 0x20F8F1: io_loop (poll.c:445)
```
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This was measured as a 95th percentile in our rough testing, thanks to
all the volunteers who monitored my channels.
Fixes: #4761
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Added: JSON-RPC: `setchannelfee` gives a grace period (`enforcedelay`) before rejecting old-fee payments: default 10 minutes.
Currently it will be used for onion replies, but we can use it for offers
and invoices in future, if we want to avoid revealing our node_id.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This expects the caller to create the TLVs to put in each hop; it
simply creates the onion and sends it.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We put this in reply paths, so we can tell if they are used. This lets us
avoid responding unless the correct reply path is used.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
One change from the obsolete version handling, gossipd will no longer send
forwarding onion msgs to lightningd, but will forward it directly.
That was the effect before, anyway.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
sendonionmessage is going to be the new one, and do much *less*.
As this is an internal experimental-only API, no deprecation cycle
required.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
offers contain an x-only pubkey: to route to them to need to know the
02 vs 03 prefix. If they're in the gossmap it's easy, but if they're
a directly-connected peer it's harder. We used to have
sendonionmessage tweak the key if it found a peer with the matching
key, but this was always a hack.
It turns out that we try to connect to the node anyway, which is
a noop if it's already connected. So try connecting to the other
parity if the first one fails.
Also, this registers when we fail to connect, and returns an error
rather than waiting for timeout.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
```
E - Node /tmp/ltests-uf2g_5gd/test_sendinvoice_obsolete_1/lightning-1/ has memory leaks: [
E {
E "backtrace": [
E "ccan/ccan/tal/tal.c:442 (tal_alloc_)",
E "ccan/ccan/tal/tal.c:471 (tal_alloc_arr_)",
E "ccan/ccan/tal/str/str.c:91 (tal_vfmt_)",
E "ccan/ccan/tal/str/str.c:44 (tal_fmt_)",
E "common/wireaddr.c:232 (fmt_wireaddr_without_port)",
E "common/wireaddr.c:251 (fmt_wireaddr)",
E "common/wireaddr.c:208 (fmt_wireaddr_internal)",
E "common/wireaddr.c:221 (fmt_wireaddr_internal_)",
E "common/type_to_string.c:32 (type_to_string_)",
E "lightningd/peer_control.c:1433 (json_add_peer)",
E "lightningd/peer_control.c:1481 (json_listpeers)",
E "lightningd/jsonrpc.c:627 (command_exec)",
E "lightningd/jsonrpc.c:762 (rpc_command_hook_final)",
E "lightningd/plugin_hook.c:274 (plugin_hook_call_)",
E "lightningd/jsonrpc.c:850 (plugin_hook_call_rpc_command)",
E "lightningd/jsonrpc.c:949 (parse_request)",
E "lightningd/jsonrpc.c:1040 (read_json)",
E "ccan/ccan/io/io.c:59 (next_plan)",
E "ccan/ccan/io/io.c:435 (io_do_always)",
E "ccan/ccan/io/poll.c:300 (handle_always)",
E "ccan/ccan/io/poll.c:377 (io_loop)",
E "lightningd/io_loop_with_timers.c:21 (io_loop_with_timers)",
E "lightningd/lightningd.c:1112 (main)"
E ],
E "label": "common/wireaddr.c:232:char[]",
E "parents": [
E "common/json_stream.c:22:struct json_stream",
E "ccan/ccan/io/io.c:91:struct io_conn",
E "lightningd/lightningd.c:103:struct lightningd"
E ],
E "value": "0x56041b322a48"
E }
E ]
```
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
After recent header files clean-up it was not possible to
build c-lightning 7401b2682. This patch fixes it both for
Alpine Linux and OpenBSD.
Proposed-by: nathanael <nathanael@dalliard.ch>
Changelog-None
Before:
Ten builds, laptop -j5, no ccache:
```
real 0m36.686000-38.956000(38.608+/-0.65)s
user 2m32.864000-42.253000(40.7545+/-2.7)s
sys 0m16.618000-18.316000(17.8531+/-0.48)s
```
Ten builds, laptop -j5, ccache (warm):
```
real 0m8.212000-8.577000(8.39989+/-0.13)s
user 0m12.731000-13.212000(12.9751+/-0.17)s
sys 0m3.697000-3.902000(3.83722+/-0.064)s
```
After:
Ten builds, laptop -j5, no ccache: 8% faster
```
real 0m33.802000-35.773000(35.468+/-0.54)s
user 2m19.073000-27.754000(26.2542+/-2.3)s
sys 0m15.784000-17.173000(16.7165+/-0.37)s
```
Ten builds, laptop -j5, ccache (warm): 1% faster
```
real 0m8.200000-8.485000(8.30138+/-0.097)s
user 0m12.485000-13.100000(12.7344+/-0.19)s
sys 0m3.702000-3.889000(3.78787+/-0.056)s
```
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This is best-practice (to ensure prototypes match up), but there were a
few places we didn't (at least, directly). Make it a requirement,
either of form "foo.h" or <dir/foo.h>.
The noise is the change to our print templates.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This allows cmdline users to have more idea what's going on.
Inspired-by: https://github.com/ElementsProject/lightning/issues/4777
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Added: `close` now notifies about the feeranges each side uses.
Currently we actually insist it's the default, but in future it could be
different.
We also need to tell openingd what the channel_type was, if we resume
via openingd_funder_complete().
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Openingd can query them itself (as dualopend already does). And move
the two feature args next to each other on the wire.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
That was quick!
We remove the 50% test, since the default is now to use quickclose.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Added: Protocol: We now perform quick-close if the peer supports it.
This affects the range we offer even without quick-close, but it's
more critical for quick-close.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Added: JSONRPC: `close` now takes a `feerange` parameter to set min/max fee rates for mutual close.
This follows https://github.com/lightningnetwork/lightning-rfc/pull/847.
For anchor_outputs, we pass down a max_feerate to closingd, and set the
fee ceiling to MAX. It uses that to estimate the desired closing fee.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-EXPERIMENTAL: Anchor output mutual close allow a fee higher than the final commitment transaction (as per lightning-rfc #847)
Based on a commit by @niftynei, but:
- Separated quickclose logic from main loop.
- I made it indep of anchor_outputs, use and option instead.
- Disable if they've specified how to negotiate.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
It also gets rid of the requirement that close negotiation fee maximum
is the old commitment transaction. We still do that, however, to
avoid surprising old peers.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This touches a lot of text, mainly to change "if `option_anchor_outputs`"
to "if `option_anchors`"
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This includes the new bolt11 test vectors, and also removes the
requirement that HTLCs be less than 2^32 msat. We keep that for now
because Electrum enforced it on receive: in two releases we will stop
that too.
So no longer warn about needing mpp in that case either.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Deprecated: Protocol: No longer restrict HTLCs to
Not necessary yet, but it will be once shutdown starts waiting for
plugins to respond: we don't want these to try to access the bcli
plugin once it's freed.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
It's a legacy from when it didn't have an ld pointer to access ld's
timer structure. Now it's just confusing.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
If commitment_revocation hook is not being used for a whatchtower but
something else, the channel_id is missing.
Changelog-Added: addes channel_id and commitnum to commitment_revocation hook
Changelog-Changed: Change order parameters in the listforwards command
Changelog-Deprecated: Change order of the status parameter in the listforwards rpc command.
Signed-off-by: Vincenzo Palazzo <vincenzopalazzodev@gmail.com>
Signed-off-by: Vincenzo Palazzo <vincenzopalazzodev@gmail.com>
Changelog-Deprecated: RPC framwork now require the "jsonrpc" propriety inside the request.
Changelog-Fixed: RPC framwork now required the "jsonrpc" propriety to be specified inside each request.