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>
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>
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>
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.
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.
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>
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>
This was causing journal_entries to show up in the accountant plugin,
since we don't emit events for unconfirmed events until they're actually
confirmed onchain.
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.
Only shows up on delayed to us outputs, but nice to have anyway.
It's missing for channel index destined deposits, maybe nice to add at
some point in the future; right now you can figure out which close a
wallet deposit comes from via the channel close txid
There is no "wallet_lib_headers" variable in wallet/Makefile
Likewise, there were two "lightningd_headers", a couple of unused
variables and some other nonsene in lightningd/Makefile
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
If this field is missing for whatever reason (weird db state?)
clightning will crash when listing invoices.
Signed-off-by: William Casarin <jb55@jb55.com>
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>
Otherwise we get weird effects, as htlcs are being freed:
```
2022-01-26T05:07:37.8774610Z lightningd-1: 2022-01-26T04:47:48.770Z DEBUG 030eeb52087b9dbb27b7aec79ca5249369f6ce7b20a5684ce38d9f4595a21c2fda-chan#8: Failing HTLC 18446744073709551615 due to peer death
2022-01-26T05:07:37.8775287Z lightningd-1: 2022-01-26T04:47:48.770Z **BROKEN** 030eeb52087b9dbb27b7aec79ca5249369f6ce7b20a5684ce38d9f4595a21c2fda-chan#8: Neither origin nor in?
```
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
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.
We want to stream gossip through this, but currently connectd treats the
fd as synchronous. While we work on getting rid of that, it's easiest to
have two fds.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Once we send funding_locked, gossipd could start seeing channel_updates
from the peer (which get sent so we can use the channel in routehints
even before it's announcable).
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We also no longer strip the type off: everyone handles both forms, and
Eclair doesn't strip (and it's easier!).
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We want it to keep the latest, so it can make its own error msgs without
asking us. This installs (but does not use!) the message handler.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This is in preparation for gossipd feeding us the latest channel_updates,
rather than having lightningd and channeld query gossipd when it wants
to send an onion error with an update included.
This means gossipd will start telling us the updates, so we need the
channels loaded first.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This fixes lightningd's chronic weight underestimate.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Fixed: closingd: more accurate weight estimation helps mutual closing near min/max feerates.
If a coin move concerns an external account, it's really useful to know
which 'internal' account initiated the transfer.
We're about to add a notification for withdrawals, so we can use this to
track wallet pushes to outside addresses
Changelog-Added: JSONRPC: `coin_movement` to 'external' accounts now include an 'originating_account' field
lightningd would race with the subd destructor to do the waitpid(),
resulting in UNUSUAL log messages, but also us missing if a plugin
was killed via a signal.
We can also get rid of the gratuitous waitpid() in test_subdaemons.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
msg_queue was originally designed for inter-daemon comms, and so it has
a special mechanism to mark that we're trying to send an fd. Unfortunately,
a peer could also send such a message, confusing us!
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
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>
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>
We actually intercept the gossip_timestamp_filter, so the gossip_store
mechanism inside the per-peer daemon never kicks off for normal connections.
The gossipwith tool doesn't set OPT_GOSSIP_QUERIES, so it gets both, but
that only effects one place.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
As connectd handles more packets itself, or diverts them to/from gossipd,
it's the only place we can implement the dev_disconnect logic.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
1. tal_strndup(.., str, strlen(str)) == tal_strdup()
2. tal_strdup also takes(), so document that.
3. Avoid passing 'struct sha256' on the stack: use ptr.
4. Generally, structures shouldn't keep pointers to things they don't own.
In this case, mvt->node_id.
5. Make payment_hash a pointer, since NULL is more natural than an all-zero
hash.
And add NON_NULL_ARGS() to the functions; it's cumbersome, but make it
fairly clear what params are optional.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
connectd is going to end up using this do demux; make it fast and complete.
Fixing this reveals a problem in openingd: it now extracts the channel_id
from funding_signed (which is where we transition off the temporary), and
gets upset. So fix that.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
1. Freeing an unconfirmed channel already releases the subd, so don't
do that explicitly.
2. Use channel->owner to transfer ownership where possible, using
channel_set_owner() which handles all the cases.
This simplifies the code and makes it more readable, IMHO.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Fire off a snapshot of current account balances (node wallet + every
'active' channel) after we've caught up to the chain tip for the *first*
time (in other words, on start).
We need to stash/save the amount of the lease fees on a leased channel,
we do this by re-using the 'push' amount field on channel (which is
technically correct, since we're essentially pushing the fee amount to
the peer).
Also updates a bit of how the pushes are accounted for (pushed to now
has an event; their channel will open at zero but then they'll
immediately register a push event).
Leases fees are treated exactly the same as pushes, except labeled
differently.
Required adding a 'lease_fee' field to the inflights so we keep track of
the fee for the lease until the open happens.
If we initialized the payment, the fees are the entire fee-chain
(final hop amount - starting hop amount)
If it's a payment we routed, the fees are the diff between the
inbound htlc and the outbound (net gain by this routing)
Added to database so data persists nicely.
We record the amount of fees collected for a routed payment. For
simplicity's sake on the data agg side, we record the fee payment on
*BOTH* the incoming htlc and the outgoing htlc. Note that this results
in double counting if you add up the fees from both an in-routed and
out-routed payment.
Get rid of the 'movement_idx', since we replay events now.
Since we're removing a field from the 'coin_movement' event emission, we
bump the version type.
Changelog-Updated: `coin_movements` events have been revamped and are now on version 2.
we used this originally to suppress duplicate issuance of coin-move
events; we're assuming that any plugin expects duplicate events though
(and knows how to de-dupe them), so we no longer need this logic.
The old model of coin movements attempted to compute fees etc and log
amounts, not utxos. This is not as robust, as multi-party opens and dual
funded channels make it hard to account for fees etc correctly.
Instead, we move towards a 'utxo' view of the onchain events. Every
event is either the creation or 'destruction' of a utxo. For cases where
the value of the utxo is not (fully) debited/credited to our account, we
also record the output_value. E.g. channel closings spend a utxo who's
entire value we may not own.
Since we're now tracking UTXOs onchain, we can now do more complex
assertions about the onchain footprint of them. The integration tests
have been updated to now use more 'chain aware' assertions about the
ending state.
we assume that every event coming from onchaind is for that channel's
account, but now that we sometimes track external + wallet events from
onchaind, we should only add the channel account name if there's nothing
set there already
We're not going to do 'spend tracks' any more; instead we'll emit an
event whenever an output is included in a broadcast tx
(even if the broadcast fails!!)
Suggested-by: Rusty Russell
Signed-off-by: Vincenzo Palazzo <vincenzopalazzodev@gmail.com>
Changelog-Changed: Support hsm specific error error code in lightning-cli
The only thing that needs ld->wallet after this is destroy_invoices_waiter (off jsonrpc)
Could not find any other destructors (destroy_*) that need wallet or db access after this.
Any db access would now segfault.
The idea is to have different default ports for different networks.
Current default port is `9735` for everything. Let's use it for
the mainnet and reuse the difference added to the default port
from `rpc_port` values in `bitcoin/chainstate.c`.
Testnet would be `19735` (adding rpc_port - 8332 = `10000`).
Signet would be `39735` (adding rpc_port - 8332 = `30000`).
Regtest would be `19846` (adding rpc_port - 8332 = `10111`).
With Vincenzo's kind pair-programming help over tmate.
Two other commits were squashed into this one so that bisecting
never ends up in half-baked state:
1. chainparams: Fix regtest default rpc_port
bitcoind -help says this:
-rpcport=<port>
Listen for JSON-RPC connections on <port> (default: 8332, testnet:
18332, signet: 38332, regtest: 18443)
2. test_gossip: Default port for regtest
hex: 2607 is now .... (could be 4d86 but Elements uses another port)
dec: 9735 is now any port (could be 19846 ^^ but now is for any port)
The lines which were binding to default port were removed as the
default port is different on each network.
NOTE: Remember not to modify gossip_store tests which loads everything raw
including the checksums.
Changelog-Changed: If the port is unspecified, the default port is chosen according to used network similarly to Bitcoin Core.
And turn "" includes into full-path (which makes it easier to put
config.h first, and finds some cases check-includes.sh missed
previously).
config.h sets _GNU_SOURCE which really needs to be done before any
'#includes': we mainly got away with it with glibc, but other platforms
like Alpine may have stricter requirements.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This has been here for a while: self_id hangs around while we're
calling the hook, but now it triggers sometimes.
```
E ValueError:
E Node errors:
E Global errors:
E - Node /tmp/ltests-3mcyp67u/test_dev_rawrequest_1/lightning-1/ has memory leaks: [
E {
E "backtrace": [
E "ccan/ccan/tal/tal.c:442 (tal_alloc_)",
E "gossipd/gossipd_wiregen.c:528 (fromwire_gossipd_got_onionmsg_to_us)",
E "lightningd/onion_message.c:152 (handle_onionmsg_to_us)",
E "lightningd/gossip_control.c:137 (gossip_msg)",
E "lightningd/subd.c:548 (sd_msg_read)",
E "ccan/ccan/io/io.c:59 (next_plan)",
E "ccan/ccan/io/io.c:407 (do_plan)",
E "ccan/ccan/io/io.c:417 (io_ready)",
E "ccan/ccan/io/poll.c:453 (io_loop)",
E "lightningd/io_loop_with_timers.c:21 (io_loop_with_timers)",
E "lightningd/lightningd.c:1164 (main)"
E ],
E "label": "gossipd/gossipd_wiregen.c:528:struct secret",
E "parents": [
E "lightningd/onion_message.c:149:struct onion_message_hook_payload",
E "lightningd/plugin_hook.c:81:struct hook_instance *[]"
E ],
E "value": "0x55cf3cbc9458"
E }
E ]
```
As of 2b923a0367c5f9154fcec706e3302cc4658dd889.
Recurrence quotes need to be marked separately, since they're no longer
in offers main bolt.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
It's very similar to the previous, but there are a few changes:
1. The enctlv fields are numbered differently.
2. The message itself is a different number.
The onionmsg_path type is the same, however, so we keep that constant
at least.
The result is a lot of cut & paste, but we will delete the old one
next release.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This is from 6e99c5feaf60cb797507d181fe583224309318e9
We renamed the enctlv field to encrypted_recipient_data in the spec, and the
new onion_message is message 513. We don't handle it until the next patch.
Two renames:
1. blinding_seed -> blinding_point.
2. enctlv -> encrypted_recipient_data.
We don't do a compat cycle for our JSON APIs for these experimental
features only used by our own plugins, we just rename.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Temporarily disable sendpay_blinding test which uses obsolete onionmsg;
there's still some debate on the PR about how blinded HTLCs will work.
Changelog-EXPERIMENTAL: onionmessage: removed support for v0.10.1 onion messages.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Setting SIGCHLD back to default (i.e. ignored) makes waitpid hang on an
old SIGCHLD that was still in the pipe?
This happens running test_important_plugin with developer=1:
(or with dev=0 and build-in plugins subscribed to "shutdown")
0 0x00007ff8336b6437 in __GI___waitpid (pid=-1, stat_loc=0x0, options=1) at ../sysdeps/unix/sysv/linux/waitpid.c:30
1 0x000055fb468f733a in sigchld_rfd_in (conn=0x55fb47c7cfc8, ld=0x55fb47bdce58) at lightningd/lightningd.c:785
2 0x000055fb469bcc6b in next_plan (conn=0x55fb47c7cfc8, plan=0x55fb47c7cfe8) at ccan/ccan/io/io.c:59
3 0x000055fb469bd80b in do_plan (conn=0x55fb47c7cfc8, plan=0x55fb47c7cfe8, idle_on_epipe=false) at ccan/ccan/io/io.c:407
4 0x000055fb469bd849 in io_ready (conn=0x55fb47c7cfc8, pollflags=1) at ccan/ccan/io/io.c:417
5 0x000055fb469bfa26 in io_loop (timers=0x55fb47c41198, expired=0x7ffdf4be9028) at ccan/ccan/io/poll.c:453
6 0x000055fb468f1be9 in io_loop_with_timers (ld=0x55fb47bdce58) at lightningd/io_loop_with_timers.c:21
7 0x000055fb46924817 in shutdown_plugins (ld=0x55fb47bdce58) at lightningd/plugin.c:2114
8 0x000055fb468f7c92 in main (argc=22, argv=0x7ffdf4be9228) at lightningd/lightningd.c:1195
since PR #3867 utxos are unreserved by height, destroy_utxos and
related functions are not used anymore so clean them up also
However free(ld->jsonrpc) still needs to happen before free(ld) because its
destructors need list_head pointers from ld
because:
- shutdown_subdaemons can trigger db write, comments in that function say so at least
- resurrecting the main event loop with subdaemons still running is counter productive
in shutting down activity (such as htlc's, hook_calls etc.)
- custom behavior injected by plugins via hooks should be consistent, see test
in previous commmit
IDEA:
in shutdown_plugins, when starting new io_loop:
- A plugin that is still running can return a jsonrpc_request response, this triggers
response_cb, which cannot be handled because subdaemons are gone -> so any response_cb should be blocked/aborted
- jsonrpc is still there, so users (such as plugins) can make new jsonrpc_request's which
cannot be handled because subdaemons are gone -> so new rpc_request should also be blocked
- But we do want to send/receive notifications and log messages (handled in jsonrpc as jsonrpc_notification)
as these do not trigger subdaemon calls or db_write's
Log messages and notifications do not have "id" field, where jsonrpc_request *do* have an "id" field
PLAN (hypothesis):
- hack into plugin_read_json_one OR plugin_response_handle to filter-out json with
an "id" field, this should
block/abandon any jsonrpc_request responses (and new jsonrpc_requests for plugins?)
Q. Can internal (so not via plugin) jsonrpc_requests called in the main io_loop return/revive in
the shutdown io_loop?
A. No. All code under lightningd/ returning command_still_pending depends on either a subdaemon, timer or
plugin. In shutdown loop the subdaemons are dead, timer struct cleared and plugins will be taken
care of (in next commits).
fixup: we can only io_break the main io_loop once
Incase the error handling happening after the quted line is non-critical:
```
return tal_fmt(NULL, "Unable to parse address '%s': %s", arg, err_msg);
```
we should not expand the proposed_listen_announce array without adding
a proposed_wireaddr. So we move the expand of proposed_listen_announce
to the location where we also expand the proposed_wireaddr.
Changelog-None
Do proper refcounting on log prefixes; previously we kept them around,
which is fine, but the extra notleak() and backtrace in developer mode
could get quite heavy (I ended up with 1G of backtraces!). This is
mainly due to creating one on every JSONRPC command, and running
clboss.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Fixed: lightningd: remove slow memory leak in DEVELOPER builds.
```
cc lightningd/subd.c
lightningd/subd.c:216:7: error: expected identifier or '('
int stdout = STDOUT_FILENO, stderr = STDERR_FILENO;
^
/usr/include/stdio.h:198:17: note: expanded from macro 'stdout'
^
lightningd/subd.c:216:7: error: expected ')'
/usr/include/stdio.h:198:17: note: expanded from macro 'stdout'
^
lightningd/subd.c:216:7: note: to match this '('
/usr/include/stdio.h:198:16: note: expanded from macro 'stdout'
^
lightningd/subd.c:224:12: error: cannot take the address of an rvalue of type 'FILE *' (aka 'struct __sFILE *')
fds[1] = &stdout;
^~~~~~~
lightningd/subd.c:225:12: error: cannot take the address of an rvalue of type 'FILE *' (aka 'struct __sFILE *')
fds[2] = &stderr;
^~~~~~~
4 errors generated.
gmake: *** [Makefile:279: lightningd/subd.o] Error 1
```
Changelog-None: introduced since last release.
Fixes: #4914
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au> ```
This loads up 20MB of plugins temporarily; we seem to be getting OOM
killed under CI and I wonder if this is contributing.
Doesn't significantly reduce runtime here, but I have lots of memory.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This surprised me, since the CHANGELOG for [0.8.2] said:
We now announce multiple addresses of the same type, if given. ([3609](https://github.com/ElementsProject/lightning/pull/3609))
But it lied!
Changelog-Fixed: We really do allow providing multiple addresses of the same type.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
October was the date Torv2 is no longer supported by the Tor Project;
it will probably not work at all by next release, so we should remove
it now even though it's not quite the 6 months we prefer for
deprecation cycles.
I still see 110 nodes advertizing Torv2 (vs 10,292 Torv3); we still
parse and display it, we just don't advertize or connect to it.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
It's both complex and flawed, as ZmnSCPxj points out. Make a generic
fd ordering routine, and use it.
Plus, test it!
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
If we're over the dust limit, we fail it immediatey *after* commiting
it, but we need a way to signal this throughout the lifecycle, so we add
it to htlc_in struct and persist it through to the database.
If it's supposed to be failed, we fail after the commit cycle is
completed.
To reduce the surface area of amount of a channel balance that can be
eaten up as htlc dust, we introduce a new config
'--max-dust-htlc-exposure-msat', which sets the max amount that any
channel's balance can be added as dust
Changelog-Added: config: new option --max-dust-htlc-exposure-msat, which limits the total amount of sats to be allowed as dust on a channel
Fixes: #4868
ChangeLog-Fixed: We now no longer self-limit the number of file descriptors (which limits the number of channels) in sufficiently modern systems, or where we can access `/proc` or `/dev/fd`. We still self-limit on old systems where we cannot find the list of open files on `/proc` or `/dev/fd`, so if you need > ~4000 channels, upgrade or mount `/proc`.
This also inadvertently fixes a latent bug: before this patch, in the
`subd` function in `lightningd/subd.c`, we would close `execfail[1]`
*before* doing an `exec`.
We use an EOF on `execfail[1]` as a signal that `exec` succeeded (the
fd is marked CLOEXEC), and otherwise use it to pump `errno` to the
parent.
The intent is that this fd should be kept open until `exec`, at which
point CLOEXEC triggers and close that fd and sends the EOF, *or* if
`exec` fails we can send the `errno` to the parent process vua that
pipe-end.
However, in the previous version, we end up closing that fd *before*
reaching `exec`, either in the loop which `dup2`s passed-in fds (by
overwriting `execfail[1]` with a `dup2`) or in the "close everything"
loop, which does not guard against `execfail[1]`, only
`dev_disconnect_fd`.
If the port is set, we spawn it (lightning_websocketd) on any
connection to that port. That means websocketd is a per-peer daemon,
but it means every other daemon uses the connection normally (it's
just actually talking to websocketd instead of the client directly).
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This makes init a two-stage, and causes some code hoisting.
And we can now send all the HTLCs in a single message, since we have
an 128MB limit and each HTLC is 37 bytes.
This breaks the onchaind stresstest, which uses canned internal messages.
It's time to finally delete that.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This re-establishes the prior behavior where a `sendpay` or
`sendonion` that'd match a prior payment would cause the prior payment
to be deleted. While we no longer delete prior attempts we now avoid a
primary key collision by incrementing once. This helps us not having
to touch all existing tests, and likely avoids breaking other users
too.
One of the fundamental constraints of the payment groups idea is that
there may only ever be one group in flight at any point in time, so if
we find a group that is in flight, any new `sendpay` or `sendonion`
must match its `groupid`.
This was the main cause of the pay states flip-flopping, since we
reset the status on each attempt any final status is not really
final. Let's keep them around, and provide a stable history.
It's probably not worth fixing for the other daemons.
Changelog-Changed: JSON-RPC: `ping` now only works if we have a channel with the peer.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
To minimize the diffs, we #if 0 the code. We'll reenable it once
channeld is ready.
We also temporarily disable the ping tests.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
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.
After some discussion with @shesek, and my own usage, we agreed that
a more comprehensive interface, which explicitly supports grouping,
is desirable.
Thus keys are now arrays, with the semantic that a key is either a
parent or has a value, never both.
For convenience in the JSON schema, we always return them as arrays,
though we accept simple strings as arguments.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We add a generation counter, and allow update or del conditional
on a given generation.
Formalizes error codes, too, since we have more now.
Suggested-by: @shesek
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Added: JSON-RPC: `datastore`, `deldatastore` and `listdatastore` for plugins to store simple persistent key/value data.
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'
This actually caused the flake in test_funding_reorg_private, where
l1 and l2 might not mark the original channel disabled. In fact, they
should *remove* it as it gets reorged out.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
dualopend doesn't always listen to lightningd messages, so it would
sometimes hang at the end of tests.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
1. We assumed an empty upfront_shutdown_script TLV would become NULL:
RPC call failed: method: fundchannel, payload: {'id': '022d223620a359a47ff7f7ac447c85c46c923da53389221a0054c11c1e3ca31d59', 'amount': 1000000, 'announce': True}, error: {'code': -1, 'message': 'They sent error channel e7c2d5d14462fe269631418fbfc3db327843382e6a2a5a9c2991d2d6ba31d9f5: Unacceptable upfront_shutdown_script ', 'data': {'id': '022d223620a359a47ff7f7ac447c85c46c923da53389221a0054c11c1e3ca31d59', 'method': 'fundchannel_start'}}"
2. We were assuming an empty enctlv would become NULL, too.
We should not have done this (there's a semantic difference between
"empty" and not-present for TLVs), so prepare for the change.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
As requested by @shesek: it's weird to fail if they ask for the exact
same thing (which is quite possible, since offers don't expire by
default).
And add a new "created" field so they can tell if they have an old
one.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Since we now use 'compact_lease' to gate an open (if the rates have
changed, we fail), we no longer need to rely on query rates for figuring
things out, so we make it dev-only.
Changelog-Changed: JSON-API: queryrates is now developer only
We need to know what the lease we're expecting is. To do this
we pass around the hex encoded portion of the wire format.
We can use this passed in expected lease rates to confirm that the peer
is, in fact, using the same rates as what we have currently.
Changelog-Added: JSON-RPC: fundchannel, multifundchannel, and openchannel_init now accept a 'compact_lease' for any requested funds
By default, we won't close a channel that we leased to a peer.
You can override this with the `force_lease_closed` flag.
Changelog-Added: JSON-RPC: close now has parameter to force close a leased channel (option_will_fund)
Typically we forget a channel if 2016 blocks have passed and
the funding transaction hasn't been mined yet, however we
SHOULD NOT forget these channels if we've got funds in them!
Useful for parsing a passed in feerate before calling lightningd with
it, e.g. when you need to know what the feerate is for a fundpsbt before
calling fundpsbt
Changelog-Added: JSON-RPC: new command `parsefeerate` which takes a feerate string and returns the calculated perkw/perkb
When a request comes through, we forward it over to the funder who
uses the currently set policy to figure out how to handle it.
Includes small update to the policy engine which decides whether or not
to fund a request.
Changelog-Experimental: Plugins: `openchannel2` hook now includes optional fields for a channel lease request
Using a 'feestep' is more restrictive than you'd want, instead we
enforce that the next feerate must be at least 1/64th more than the
last, but put no upper limit on it
Includes update to lnprototest changes
Contributed-By: Vincenzo Palazzo <vincenzopalazzodev@gmail.com>
Changelog-EXPERIMENTAL: Protocol: Replaces init_rbf's `fee_step` for RBF of v2 opens with `funding_feerate_perkw`, breaking change
This adds a `u32 checksum` field to the plugin struct that is used
to identify if a plugin is outdated and needs to be restarted on `rescan`.
Note: Only affects non-important plugins.
Changelog-Added: Plugin: Restart plugin on `rescan` when binary was changed.
Incoming HTLCs are rejected by the HTLC logic if the payload contains
an even type that `lightningd` doesn't recognize. This is to prevent
us from accidentally accepting a payment that has extra semantics
attached (for example if we get a keysend payment and don't know what
to do with the TLV field containing the message we should reject it,
otherwise the overall semantics of the message delivery fail).
Enable non-dev builds to send custom messages.
Preserves 'dev-' for compat-enabled builds.
Changelog-Changed: JSON-RPC: moved dev-sendcustommsg to sendcustommsg
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>
This makes it easier to access (rather than decoding bolt11).
Changelog-Added: JSON-RPC: `invoice` now outputs explicit `payment_secret` it its own field.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Useful for regtest and testnet. Sure, you shouldn't use this on mainnet,
but I haven't restricted it because our users are usually pretty clever.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Fixes: #1806
Changelog-Added: config: `force_feerates` option to allow overriding feerate estimates (mainly for regtest).
We don't do it for sendinvoice (yet?).
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-EXPERIMENTAL: `fetchinvoice` can take a payer note, and `listinvoice` will show the payer_notes received.
We don't support it (yet), but update the spec to include it.
We include the previous field (recurrence_signature) as a shim for the
moment, for compat with existing nodes. It's ugly, but next release
we'll stop *sending* it, then finally we'll stop accepting it!
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
As per latest spec revision.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-EXPERIMENTAL: BOLT12 offers can now be unsigned, for really short QR codes.
Discovered by the next test, oops.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Fixed: JSONRPC: `listinvoice` no longer crashes if given an invalid (or bolt12) `invstring` argument.
Rusty Russell
Ken Sedgwick
Michael Schmoock
niftynei
Simon Vrouwe
William Casarin
Anand Suresh
Vincenzo Palazzo
ZmnSCPxj jxPCSnmZ
Jan Sarenik
Christian Decker