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.
We were actually using the last commit tx's size, since we were
setting it in lightningd. Instead, hand the min and desired feerates
to closingd, and (as it knows the weight of the closing tx), and have
it start negotiation from there.
This can be significantly less when anchor outputs are enabled: for
example in test_closing.py, the commit tx weight is 1124 Sipa, the
close is 672 Sipa!
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Changed: Protocol: Use a more accurate fee for mutual close negotiation.
Show amount they were trying to pay with, not invoice amount.
Also, show min fee in closing, not fee they offered.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
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).
This lets us transition (with a few supporting changes) to closingd,
which will happily let them mutual close with us.
We already handle the case where this mutual close is redundant (for
packet loss), so this is easy.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Added: Protocol: We will now reestablish and negotiate mutual close on channels we've already closed (great if peer has lost their database).
This supports reestablish on a closed channel: we tell channeld to
respond to the reestablish message appropriately, then close the
channel.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
It handles all the cases of retransmission, and in the normal case
retransmits shutdown and immediately returns for us to run closingd.
This is actually far simpler and reduces code duplication.
[ Includes fixup to stop warn_unused_result from Christian ]
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Fixed: Protocol: We could get stuck on signature exchange if we needed to retransmit the final revoke_and_ack.
1. Hide the now-deprecated enable-autotor-v2-mode option.
2. Really don't print dev- options.
3. Don't print true and false as strings in some cases.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Fixed: JSON-RPC: `listconfigs` would list some boolean options as strings `"true"` or `"false"` instead of using JSON booleans.
Makes it possible to write a decent JSON schema, but means we need to carry
additional data, so we create a `struct plugin_command`.
We remove the unused struct dynamic_plugin, too.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
In general, it's better to omit a field than put in a 'null', and
putting variable-named fields in an object is also a bad idea.
This is reflected in how hard this is to express in JSON schema, too.
Others:
1. Remove the obsolete "funding": "LOCAL" from unopened channels, but add
"opener": "local" as used in normal channels.
2. htlc cltv_expiry is a u16.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Deprecated: JSON-RPC: `listfunds` `channels` `funding_allocation_msat` and `funding_msat`: use `funding`.
Changelog-Deprecated: JSON-RPC: `listfunds` `channels` `last_tx_fee`: use `last_tx_fee_msat`.
Changelog-Deprecated: JSON-RPC: `listfunds` `channels` `closer` is now omitted if it does not apply, not JSON `null`.
https://github.com/lightningnetwork/lightning-rfc/pull/877 talks about
removing this restriction (only Electrum actually enforced it on
receive), so start by allowing creation of giant invoices, though
we mark them as requiring mpp.
Changelog-Changed: JSON-RPC: `invoice` now allows creation of giant invoices (>= 2^32 msat)
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This mainly helps our CI under valgrind, which starts a fresh instance
and immediately calls the invoice command. This can cause the topology
plugin to try to access the gossmap file before it's created.
We can also move the gossmap reading in topology to init time.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This involves removing some fields from the now-misnamed routing.h
datastructures, and various internal messages.
One non-obvious change is to our "keepalive" logic which refreshes
channels every 13 days: instead of using the 'enabled' flag on the
last channel broadcast to decide whether to refresh it, we use the
local connected status directly.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This turned into a more extensive cleanup than intended. The previous
warnings were overlapping and confusing, especially now MPP is the norm.
*warning_capacity* is now the "even under best circumstances, we don't
have enough incoming capacity", which is really what
warning_mpp_capacity was trying to say (no longer printed).
*warning_offline* and *warning_deadends* are only given if adding such
peers would have helped give capacity (i.e. not if *warning_capacity*
is set). The new *warning_private_unused* tells you that we would
have sufficient capacity, but we refused to expose private channels.
The test cases have been enhanced to cover the new warnings.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Added: JSON-RPC: `invoice` now gives `warning_private_unused` if unused unannounced channels could have provided sufficient capacity.
Changelog-Changed: JSON-RPC: `invoice` warnings are now better defined, and `warning_mpp_capacity` is no longer included (since `warning_capacity` covers that).
Temporarily rename old getroute to getrouteold (we will remove this).
Changelog-Changed: JSON-RPC: `getroute` is now implemented in a plugin.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Prior to this, sending a v1 address (or, in fact, any random crap!)
would cause the unsupporting node to unilaterally close.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
An "active" channel may still be CHANNELD_AWAITING_LOCKIN, so have ->scid NULL.
You can only trigger this by trying to sendpay to the node using a
manual route, since routing would never use such a channel.
```
lightningd: FATAL SIGNAL 11 (version v0.10.0-319-g81cbc20-modded)
0x55e79d194e17 send_backtrace
common/daemon.c:39
0x55e79d194ec1 crashdump
common/daemon.c:52
0x7fce2d79920f ???
???:0
0x7fce2d8e16f7 ???
???:0
0x55e79d2019eb tal_dup_
ccan/ccan/tal/tal.c:801
0x55e79d14e1d9 immediate_routing_failure
lightningd/pay.c:365
0x55e79d14fe91 send_payment_core
lightningd/pay.c:1022
0x55e79d150995 send_payment
lightningd/pay.c:1180
0x55e79d151975 json_sendpay
lightningd/pay.c:1462
```
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Annotating the htlc in `listpeers` with their current status, and
which plugin is currently holding on to them with their
`htlc_accepted` hook can help us debug where plugins may go wrong.
Changelog-Added: jsonrpc: HTLCs in `listpeers` are now annotated with a status if they are waiting on an `htlc_accepted` hook of a plugin.
We don't actually set desired_type yet, but this handles it.
Changelog-EXPERIMENTAL: Protocol: we can now upgrade old channels to `option_static_remotekey` from https://github.com/lightningnetwork/lightning-rfc/pull/868
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
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>
Undocumented (caught by json schema!) if we discard channel because it
wasn't open yet, then close returned the empty object. Make it return
a new type in this case.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Added: JSONRPC: `close` returns `type` "unopened" if it simply discards channel instead of empty object.
Tor v2 hidden services have been deprecated for a while:
https://blog.torproject.org/v2-deprecation-timeline .
This prevents user from being able to set them in the configuration
and to connect to them while still letting us be able to parse them
for gossip.
Changelog-Deprecated: lightningd: v2 Tor addresses. Use v3. See https://blog.torproject.org/v2-deprecation-timeline.
Signed-off-by: Antoine Poinsot <darosior@protonmail.com>
If the peer is offline when we see the funding txid, we don't actually
update the channel's info. Here, we move it up to where the scid is set,
so that we always update the channel's funding_txid to the correct
(mined) information.
We set the timeout on first HTLC, but didn't clear it if that HTLC failed.
It's saner to have a per-HTLC timeout (since that's what it is!) and
also our timer infra is specially coded to scale approximately infinitely so
trying to optimize this is vastly premature.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Fixed: Protocol: We would sometimes gratuitously disconnect 30 seconds after an HTLC failed.
1. We don't need to check for NULL before tal_count(NULL).
2. Use of json_for_each_arr iterator is probably better.
3. Weird indent fixed.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Trying to put all the disconnect logic into the same path was a dumb
idea. If you asked to reconnect but passed in an 'unsaved' channel, we
would not call the 'reconnect' code.
Instead, we make a differentiation between "unsaved" channels
(ones that we haven't received commitment tx for) and handle the
disconnect for these separate from where we want to do a reconnect.
We were freeing the payload, which is then subsequently freed by the
plugin_hook caller. Whoops.
Now we pass through to the callback function and just clean up neatly.
------------------------------- Valgrind errors --------------------------------
Valgrind error file: valgrind-errors.406602
==406602== Invalid read of size 8
==406602== at 0x12AC93: openchannel2_hook_cb (dual_open_control.c:669)
==406602== by 0x12AF0A: openchannel2_hook_deserialize (dual_open_control.c:721)
==406602== by 0x16EF0E: plugin_hook_callback (plugin_hook.c:186)
==406602== by 0x169746: plugin_response_handle (plugin.c:514)
==406602== by 0x169959: plugin_read_json_one (plugin.c:620)
==406602== by 0x169B23: plugin_read_json (plugin.c:665)
==406602== by 0x1F4076: next_plan (io.c:59)
==406602== by 0x1F4C5B: do_plan (io.c:407)
==406602== by 0x1F4C9D: io_ready (io.c:417)
==406602== by 0x1F6F35: io_loop (poll.c:445)
==406602== by 0x13D48D: io_loop_with_timers (io_loop_with_timers.c:24)
==406602== by 0x143388: main (lightningd.c:1111)
==406602== Address 0x75e7418 is 56 bytes inside a block of size 3,520 free'd
==406602== at 0x483CA3F: free (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==406602== by 0x204FB0: del_tree (tal.c:421)
==406602== by 0x20527E: tal_free (tal.c:486)
==406602== by 0x122D68: delete_channel (channel.c:124)
==406602== by 0x129291: channel_disconnect (dual_open_control.c:63)
==406602== by 0x129364: channel_close_conn (dual_open_control.c:82)
==406602== by 0x131CF6: peer_please_disconnect (connect_control.c:304)
==406602== by 0x131DEB: connectd_msg (connect_control.c:326)
==406602== by 0x172023: sd_msg_read (subd.c:509)
==406602== by 0x1F4076: next_plan (io.c:59)
==406602== by 0x1F4C5B: do_plan (io.c:407)
==406602== by 0x1F4C9D: io_ready (io.c:417)
==406602== Block was alloc'd at
==406602== at 0x483B7F3: malloc (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==406602== by 0x204A39: allocate (tal.c:250)
==406602== by 0x204FFA: tal_alloc_ (tal.c:428)
==406602== by 0x123165: new_unsaved_channel (channel.c:209)
==406602== by 0x130D34: peer_start_dualopend (dual_open_control.c:2985)
==406602== by 0x15BD2A: peer_connected_hook_final (peer_control.c:1105)
==406602== by 0x16F2E5: plugin_hook_call_ (plugin_hook.c:275)
==406602== by 0x15BF5C: plugin_hook_call_peer_connected (peer_control.c:1155)
==406602== by 0x15C16C: peer_connected (peer_control.c:1208)
==406602== by 0x131E3B: connectd_msg (connect_control.c:332)
==406602== by 0x172023: sd_msg_read (subd.c:509)
==406602== by 0x171842: read_fds (subd.c:310)
Fixes: #4494
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Added: config: New option `log-timestamps` allow disabling of timestamp prefix in logs.
Since plugins will start sending them soon, and they are likely to get
it wrong sometimes, be a bit more lenient, warn them in the logs
instead and then make sure it doesn't accidentally work anyway.
A plugin might subscribe to a notification topic that is only
registered by another plugin later, so push the check to that
consistency check phase where we do hook ordering as well.
We will eventually start emitting and dispatching custom notifications
from plugins just like we dispatch internal notifications. In order to
get reasonable error messages we need to make sure that the topics
plugins are asking for were correctly registered. When doing this we
don't really care about whether the plugin that registered the
notification is still alive or not (it might have died, but
subscribers should stay up and running), so we keep a list of all
topics attached to the `struct plugins` which gathers global plugin
information.
This avoids subdaemons complaining about malformed messages from us,
or doing the completely wrong thing, if they are really the wrong
version.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
You still shouldn't do this (you could get some transient failures),
but at least you have a decent chance if you reinstall over a running
daemon, instead of getting confusing internal errors if message
formats have changed.
Changelog-Added: lightningd: we now try to restart if subdaemons are upgraded underneath us.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Fixes: #4346
We generally hang things off our JSON response (this pattern predates
tmpctx!) but sometimes it gets reported as a memleak. I'd prefer not
to mark JSON responses as "notleak", since they can be allocated for
a while), so use tmpctx here.
```
E ValueError:
E Node errors:
E Global errors:
E - Node /tmp/ltests-spnausnb/test_htlc_out_timeout_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 "wallet/wallet.c:1775 (wallet_state_change_get)",
E "lightningd/peer_control.c:922 (json_add_channel)",
E "lightningd/peer_control.c:1424 (json_add_peer)",
E "lightningd/peer_control.c:1454 (json_listpeers)",
E "lightningd/jsonrpc.c:643 (command_exec)",
E "lightningd/jsonrpc.c:767 (rpc_command_hook_final)",
E "lightningd/plugin_hook.c:275 (plugin_hook_call_)",
E "lightningd/jsonrpc.c:855 (plugin_hook_call_rpc_command)",
E "lightningd/jsonrpc.c:942 (parse_request)",
E "lightningd/jsonrpc.c:1033 (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:24 (io_loop_with_timers)",
E "lightningd/lightningd.c:1097 (main)"
E ],
E "label": "wallet/wallet.c:1775:struct state_change_entry[]",
E "parents": [
E "common/json_stream.c:29:struct json_stream",
E "ccan/ccan/io/io.c:91:struct io_conn",
E "lightningd/lightningd.c:116:struct lightningd"
E ],
E "value": "0x55c6b02150b8"
E }
E ]
```
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
The main change which affects us is that 2016 blocks to forget a channel
is a fixed number in the spec; we make this clear by renaming the
(developer-only) max_funding_unconfirmed to dev_max_funding_unconfirmed
and making it compile DEVELOPER only.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This means remembering the connection direction. We also use the address to try
to reconnect, which we shouldn't bother with if they connect to us.
For peers from the database, we currently always save the addr: we shouldn't really
do this if they connected to us, since it's not useful for reconnecting (we don't
show the addr in JSON reply to listpeers unless we're connected, so it's only an
internal issue). This is left for future work.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
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>
You can now activate dual-funded channels using the
`--experimental-dual-fund` flag
Changelog-Changed: Config: `--experimental-dual-fund` runtime flag will enable dual-funded protocol on this node
As @fiatjaf points out we were notifying before we were actually set
on accepting, since the hook could also still reject. Switched them
around does and calling the notification only once it's been decided
is the correct thing to do.
Changelog-Fixed: plugin: The `invoice_payment` notification was being sent before the hook was called, which could still abort it.
Suggested-by: Fiatjaf <@fiatjaf>
Signed-off-by: Christian Decker <@cdecker>
There are perfectly valid reasons for us to not have a command on return
(something went boom while sending them our sigs and we've now gotten
their sigs during a reconnect and subsequently broadcast the tx)
This can result in us logging a warning if we've 1) dropped their sigs
response, 2) only us (the opener) added inputs, 3) and we broadcast on
their reconnect (when they retransmit their sigs)
Otherwise, we might find an address other than the one given and
the user might think that address worked.
Fixes: #4185
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Added: JSON-RPC: `connect` returns `address` it actually connected to
And update all the in-tree callers.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Deprecated: JSON-RPC: `fundchannel_complete` `txid` and `txout` parameters (use `psbt`)
Requiring the user to calculate the txid of the PSBT is a horrible, bad,
no-good idea.
Doesn't deprecate yet, so I can test that this path works while
multifundchannel still uses it.
Fixes: #4416 (at least for future users!)
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Added: JSON-RPC: `fundchannel_complete` takes a psbt parameter.
Changelog-Added: lightningd: experimental-shutdown-wrong-funding to allow remote nodes to close incorrectly opened channels.
Changelog-Added: JSON-RPC: close has a new `wrong_funding` option to try to close out unused channels where we messed up the funding tx.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
It's not unheard of for people to give the wrong funding tx to us,
getting their funds stuck. Interestingly, we can allow mutual close
using a different txid and output number as long as they (solely)
funded the channel, and the channel hasn't been used.
This defines a "play area" feature to do just that.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Allows us to clean up an in-progress open that we won't be completing
Changelog-Added: EXPERIMENTAL JSON-RPC: Permit user-initiated aborting of in-progress opens. Only valid for not-yet-committed opens and RBF-attempts
The `rbf_channel` hook uses `our_funding_msat`, which is a nicer
and more easily understood than the `openchannel2`
`accepter_funding_msat`.
This updates the `openchannel2` hook to use the same nomenclature as
`rbf_channel`.
We were reporting the failure immediately but still continuing with
the startup. This could happen if an important plugin ends up in a
race with another plugin (important or not) for a contended
resource (CLI option or RPC method name). We would eventually notice
that we were supposed to abort, but at that point we already processed
a couple of blocks, loaded the entire state, etc.
This just aborts early with a sane error message.
Changelog-Added: plugin: If there is a misconfiguration with important plugins we now abort early with a more descriptive error message.
Reported-by: PsySc0rpi0n
Reported-by: Ján Sáreník <@jsarenik>
The receiving node: ...
- MUST fail the channel if:
- the `witness_stack` weight lowers the effective `feerate`
below the agreed upon transaction `feerate`
We consolidate to the latest/singular RFC patch for dual-funding, so
there's just a single patchfile for the change. Plus we move back to the
opener setting the desired feerate, the accepter merely declines to
participate if they disagree with the set rate.
Looks like #4394 treated a symptom but not the root cause. We were
actually sending the message framed with the WIRE_CUSTOMMSG_OUT and
the length prefix over the encrypted connection to the peer. It just
happened to be a valid custommsg...
This fixes the issue, and this time I made sure we actually send the
raw message over the wire. However for backward compatibility we
needed to imitate the faulty behavior which is 90% of this patch :-)
Changelog-Fixed: plugin: `dev-sendcustommsg` included the type and length prefix when sending a message.
We were getting a memleak error that the open_attempt isnt' being
cleaned up in test_rbf_reconnect_tx_construct. I had some trouble
reproducing it, so I removed the reliance on using `tmpctx` to clean it
up and was more surgical about cleaning it up inline.
Changelog-Added: JSON-RPC: `listpeers` now includes 'last_feerate', 'next_feerate', 'initial_feerate' and 'next_fee_step' for channels in state DUALOPEND_AWAITING_LOCKIN
fixup! listpeers: include feerate info for RBF-candidate channels
We move over to the new "warning" paradigm, instead of using
an "rbf_fail" message.
Every failure is either a warning or an error; on warnings we
hang up and reconnect later, effectively resetting the state.
We're *mostly* set up for both sides doing RBF, except that it reverses
the callback flow (using the plugin vs RPC calls) and we're not
currently smart enough to flip between them gracefully
Users have no idea what they would pay for unilateral closes.
At least this gives them a clue!
Reported-by: @az0re on IRC.
Changelog-Added: JSON-RPC: `listpeers` now shows latest feerate and unilaral close fee.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Rusty Russell
Vincenzo Palazzo
Christian Decker
Jan Sarenik
Simon Vrouwe
niftynei
Michael Schmoock
fiatjaf
openoms
Nalin Bhardwaj
Antoine Poinsot