We allow connectd to tell us a peer has gone away, but now we need
to make sure we don't double-spiderman and tell it to disconnect peer.
This is particularly harmful on reconnect: it (will soon) tell us the
old connection is gone, ready to tell us the new peer has connected.
We would tell it to disconnect the peer, which throws away the new
connection!
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This is redundant now, since connectd only sends us this once we tell
it it's OK, but that's changing, so clean up now. This means that
connectd will be able to make *unsolicited* closes, if it needs to.
We share logic with peer_please_disconnect.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
It's directly a product of "does it have a current owner subdaemon"
and "does that subdaemon talk to peers", so create a helper function
which just evaluates that instead.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
It's available in listpeers() if you want to see it, otherwise it's not
really something users want to see in the normal course of operation.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Rather than a generic "add member", provide two routines: one which
doesn't quote, and one which does.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
There are hardly any lightningd-specific JSON functions: all that's left
are the feerate ones, and there's already a comment that we should have
a lightningd/feerate.h.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We have them split over common/param.c, common/json.c,
common/json_helpers.c, common/json_tok.c and common/json_stream.c.
Change that to:
* common/json_parse (all the json_to_xxx routines)
* common/json_parse_simple (simplest the json parsing routines, for cli too)
* common/json_stream (all the json_add_xxx routines)
* common/json_param (all the param and param_xxx routines)
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This was introduced to allow creating a shared secret, but it's better to use
makesecret which creates unique secrets. getsharedsecret being a generic ECDH
function allows the caller to initiate conversations as if it was us; this
is generally OK, since we don't allow untrusted API access, but the commando
plugin had to blacklist this for read-only runes explicitly.
Since @ZmnSCPxj never ended up using this after introducing it, simply
remove it.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Removed: JSONRPC: `getsharedsecret` API: use `makesecret`
Previously we wouldn't notify when a channel moves into state
"CHANNELD_AWAITING_LOCKIN", as this is the original state (so there's
no movement btw states). This meant that it's impossible to track when a
channel's commitment txs have been exchanged and we're waiting for
onchain confirmation.
It's useful to have notice of this initialization though, all in one
place so that the `channel_state_changed` notification can successfully
track the entire lifecycle of a channel, from inception to close.
Note that for v2 "dual-funded" channels, we already notify at the same place, at
"DUALOPEND_AWAITING_LOCKIN" (the initial state for a dualopend channel
is "DUALOPEND_OPEN_INIT" -- this is the only state we don't get notified
at now...)
Changelog-Added: Plugins: `channel_state_changed` now triggers for a v1 channel's initial "CHANNELD_AWAITING_LOCKIN" state transition (from prior state "unknown")
Since it's a deprecation, we simply ignore one, rather than properly
checking they match etc.
Fixes: #5386
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
There's one case where we can present more infomation, so do that, and
fix documentation.
Changelog-Added: JSON-RPC: `listforwards` now shows `out_channel` in more cases: even if it couldn't actually send to it.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Fixes: #5329
They're not logically connected: we can know where they wanted to
go, but we didn't send it.
Where possible, it's the scid *they asked for*; otherwise, it's the
scid or fallback to the alias, but do this in the *caller*, not by
overriding inside wallet_forwarded_payment_add.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
The extra entry in opt_table would never be called, leaving plugins
clueless why options keep defaulting.
Note that option registration outside startup does nothing.
Instead, dynamic plugins can use `plugin start [second_parameter]` to pass options.
Otherwise we hangs forever in startup when it was the last plugin, we would
miss destroy_plugin --> check_plugins_manifests --> io_break
e.g. when a plugin tries to register a bool option with a string as default value.
We noticed bogus behavior where (with 200 blocks added at once)
lightningd didn't see a DF channel open: this is because we told
lightningd about the new blocks (and it timed out channel) before
the watches for the transaction are fired.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We had this assertion fail, and I can't see a clear reason why. Remove it
(i.e. don't crash because we're having trouble with creating invoice routehints)
and add logging.
```
Assertion failed: a->c->rr_number < b->c->rr_number (lightningd/invoice.c: cmp_rr_number: 623)
lightningd: FATAL SIGNAL 6 (version v0.10.2-modded)
0x5654f1c40061 send_backtrace
common/daemon.c:33
0x5654f1c400e9 crashdump
common/daemon.c:46
0x7efd87da6c8a ???
???:0
```
There are several possible causes for this:
1. We have two channels with the same rr_number. A quick audit shows we always set that rr_number to a unique value (and 64 bits, so wrap is not possible between the release and now!).
2. It's theoretically possible that sort() could compare a value with itself, but that would be really dumb: it doesn't that I've ever seen, but then, we've never seen this assert() hit, either.
3. listincoming has given us the same channel twice. I don't see how that is possible: we had a race where channels could momentarily vanish, but never be duplicated (emailed crash.log shows no duplicates!).
4. General corruption/freed memory access. If a channel we've just looked up is gone but still in the hash table, this is possible but would cause lots of random behavior and crashes.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Caused a crash in CI, reproduced under valgrind by calling
any_channel_by_scid from io_poll_lightningd:
```
==2422524== Conditional jump or move depends on uninitialised value(s)
==2422524== at 0x12C98D: any_channel_by_scid (channel.c:606)
==2422524== by 0x14FF75: io_poll_lightningd (lightningd.c:682)
==2422524== by 0x225FDE: io_loop (poll.c:420)
==2422524== by 0x14A914: io_loop_with_timers (io_loop_with_timers.c:22)
==2422524== by 0x150C4E: main (lightningd.c:1193)
==2422524== Uninitialised value was created by a heap allocation
==2422524== at 0x483B7F3: malloc (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==2422524== by 0x234F61: allocate (tal.c:250)
==2422524== by 0x235522: tal_alloc_ (tal.c:428)
==2422524== by 0x12B500: new_unsaved_channel (channel.c:203)
==2422524== by 0x13B77A: json_openchannel_init (dual_open_control.c:2610)
==2422524== by 0x14C78D: command_exec (jsonrpc.c:630)
==2422524== by 0x14CD9F: rpc_command_hook_final (jsonrpc.c:765)
==2422524== by 0x181DDA: plugin_hook_call_ (plugin_hook.c:278)
==2422524== by 0x14D198: plugin_hook_call_rpc_command (jsonrpc.c:853)
==2422524== by 0x14D6A0: parse_request (jsonrpc.c:957)
==2422524== by 0x14DAFE: read_json (jsonrpc.c:1054)
==2422524== by 0x2231C8: next_plan (io.c:59)
```
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
It actually only sets the prefix for the lightningd core log messages;
the other logs have their own prefix.
Make it a real, process-wide prefix which actually goes in front of the timestamp.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Fixed: options: `log-prefix` now correctly prefixes *all* log messages.
That's useful for "tell me everything about this node" debugging.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Fixes: #5348
Changelog-Added: lightningd: `log-level=debug:<partial-nodeid>` supported to get debug-level logs for everything about a peer.
If we can't broadcast the tx, confirm that it didn't end up in the
mempool or the utxo set before throwing an error.
Note that this doesn't protect us in the case where the funding
output has already been *spent*... but that's extremely rare, right?
Fixes#5296
Reported-By: @rustyrussell
Collab-With: @vincenzopalazzo
Check funding_outnum validity first to avoid reading invalid outputs
Changelog-Fixed: Fixed a potential issue if the number of outputs decreases in a dualopen RBF or splice.
We used to agree up on the `minimum_depth` with the peer, thus when
they told us that the funding locked we'd be sure we either have a
scid or we'd trigger the state transition when we do. However if we
had a scid, and we got a funding_locked we'd trust them not to have
sent it early. Now we explicitly track the depth in the channel while
waiting for the funding to confirm.
Changelog-Fixed: channeld: Enforce our own `minimum_depth` beyond just confirming
Not only can the outgoing edge be a zeroconf channel, it can also be
the incoming channel. So we revert to the usual trick of using the
local alias if the short_channel_id isn't known yet.
We use the LOCAL alias instead of the REMOTE alias even though the
sender likely used the REMOTE alias to refer to the channel. This is
because we control the LOCAL alias, and we keep it stable during the
lifetime of the channel, whereas the REMOTE one could change or not be
there yet.
We use this in a couple of places, when we want to refer to a channel
by its `short_channel_id`, I'm moving this into a separate function
primarily to have a way to mark places where we do that.
The direction only depends on the ordering between node_ids, not the
short_channel_id, so we can include it and it won't change. This was
causing some trouble loading the `channel_hints` in the `pay` plugin.
Rusty Russell
Michael Schmoock
Vincenzo Palazzo
niftynei
adi2011
Simon Vrouwe
Dustin Dettmer
Christian Decker