Since we now have a couple of long-lived dependents it is time we stop
removing channels from the table once they are fully closed, and instead just
mark them as closed. This allows us to keep forwards and transactions foreign
keys intact, and it may help us debug things after the fact.
Fixes#2028
Signed-off-by: Christian Decker <decker.christian@gmail.com>
Instead of deleting the channels we will simple mark them as `CLOSED` from now
on. This is needed for some of the other tables not to end up with dangling
references that would otherwise survive the channel lifetime, e.g., forwards
and transactions.
Signed-off-by: Christian Decker <decker.christian@gmail.com>
Direct leak of 16 byte(s) in 1 object(s) allocated from:
#0 0x7ff02889063e in calloc (/lib/x86_64-linux-gnu/libasan.so.5+0x10c63e)
#1 0x555ce2ad8d2e in htable_default_alloc ccan/ccan/htable/htable.c:19
#2 0x555ce2ad9698 in double_table ccan/ccan/htable/htable.c:226
#3 0x555ce2ad9b62 in htable_add_ ccan/ccan/htable/htable.c:331
#4 0x555ce2a638e4 in htlc_in_map_add lightningd/htlc_end.h:113
#5 0x555ce2a63beb in connect_htlc_in lightningd/htlc_end.c:39
#6 0x555ce2a85cbc in channel_added_their_htlc lightningd/peer_htlcs.c:1382
#7 0x555ce2a860e1 in peer_got_commitsig lightningd/peer_htlcs.c:1466
#8 0x555ce2a5db04 in channel_msg lightningd/channel_control.c:228
#9 0x555ce2a8d393 in sd_msg_read lightningd/subd.c:474
#10 0x555ce2ada157 in next_plan ccan/ccan/io/io.c:59
#11 0x555ce2adacd4 in do_plan ccan/ccan/io/io.c:407
#12 0x555ce2adad12 in io_ready ccan/ccan/io/io.c:417
#13 0x555ce2adcd67 in io_loop ccan/ccan/io/poll.c:445
#14 0x555ce2a67c66 in io_loop_with_timers lightningd/io_loop_with_timers.c:24
#15 0x555ce2a6e56b in main lightningd/lightningd.c:822
#16 0x7ff028242b6a in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x26b6a)
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Direct leak of 64 byte(s) in 1 object(s) allocated from:
#0 0x7f4dc279163e in calloc (/lib/x86_64-linux-gnu/libasan.so.5+0x10c63e)
#1 0x564ee8a24bb1 in htable_default_alloc ccan/ccan/htable/htable.c:19
#2 0x564ee8a2551b in double_table ccan/ccan/htable/htable.c:226
#3 0x564ee8a259e5 in htable_add_ ccan/ccan/htable/htable.c:331
#4 0x564ee89a5300 in block_map_add lightningd/chaintopology.h:83
#5 0x564ee89a6ece in add_tip lightningd/chaintopology.c:626
#6 0x564ee89a72c3 in have_new_block lightningd/chaintopology.c:694
#7 0x564ee89a3ab0 in process_rawblock lightningd/bitcoind.c:466
#8 0x564ee89a2fb4 in bcli_finished lightningd/bitcoind.c:214
#9 0x564ee8a284d6 in destroy_conn ccan/ccan/io/poll.c:244
#10 0x564ee8a284f6 in destroy_conn_close_fd ccan/ccan/io/poll.c:250
#11 0x564ee8a34a0d in notify ccan/ccan/tal/tal.c:235
#12 0x564ee8a34efc in del_tree ccan/ccan/tal/tal.c:397
#13 0x564ee8a35288 in tal_free ccan/ccan/tal/tal.c:481
#14 0x564ee8a26cf5 in io_close ccan/ccan/io/io.c:450
#15 0x564ee8a28c11 in io_loop ccan/ccan/io/poll.c:449
#16 0x564ee89b3c3b in io_loop_with_timers lightningd/io_loop_with_timers.c:24
#17 0x564ee89ba540 in main lightningd/lightningd.c:822
#18 0x7f4dc2143b6a in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x26b6a)
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Indirect leak of 48 byte(s) in 1 object(s) allocated from:
#0 0x7f4c84ce4448 in malloc (/lib/x86_64-linux-gnu/libasan.so.5+0x10c448)
#1 0x55d11b77d270 in strmap_add_ ccan/ccan/strmap/strmap.c:90
#2 0x55d11b704603 in command_set_usage lightningd/jsonrpc.c:891
#3 0x55d11b733cb5 in param common/param.c:295
#4 0x55d11b6f7b37 in json_connect lightningd/connect_control.c:96
#5 0x55d11b7042ef in setup_command_usage lightningd/jsonrpc.c:841
#6 0x55d11b70443b in jsonrpc_command_add_perm lightningd/jsonrpc.c:863
#7 0x55d11b704533 in jsonrpc_setup lightningd/jsonrpc.c:876
#8 0x55d11b705695 in new_lightningd lightningd/lightningd.c:210
#9 0x55d11b706062 in main lightningd/lightningd.c:644
#10 0x7f4c84696b6a in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x26b6a)
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Direct leak of 1024 byte(s) in 2 object(s) allocated from:
#0 0x7f4c84ce4448 in malloc (/lib/x86_64-linux-gnu/libasan.so.5+0x10c448)
#1 0x55d11b782c96 in timer_default_alloc ccan/ccan/timer/timer.c:16
#2 0x55d11b7832b7 in add_level ccan/ccan/timer/timer.c:166
#3 0x55d11b783864 in timer_fast_forward ccan/ccan/timer/timer.c:334
#4 0x55d11b78396a in timers_expire ccan/ccan/timer/timer.c:359
#5 0x55d11b774993 in io_loop ccan/ccan/io/poll.c:395
#6 0x55d11b72322f in plugins_init lightningd/plugin.c:1013
#7 0x55d11b7060ea in main lightningd/lightningd.c:664
#8 0x7f4c84696b6a in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x26b6a)
To fix this, we actually make 'ld->timers' a pointer, so we can clean
it up last of all. We can't free it before ld, because that causes
timers to be destroyed.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
I was working on rewriting our (somewhat chaotic) tx watching code
for 0.7.2, when I found this bug: we don't always notice the funding
tx in corner cases where more than one block is detected at
once.
This is just the one commit needed to fix the problem: it has some
unnecessary changes, but I'd prefer not to diverge too far from my
cleanup-txwatch branch.
Fixes: #2352
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
If we ever do this, we'd end up with an unspendable commitment tx anyway.
It might be able to happen if we have htlcs added from the non-fee-paying
party while the fees are increased, though. But better to close the
channel and get a report about it if that happens.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We used to produce these, but they're invalid. When we switched to
libwally it (correctly) refuses to get a txid for them.
Fixes: #2772Fixes: #2759
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
It needs this in compat mode to detect old (pre-0.6.3) end of JSON.
But it always does the first command in compat mode.
This was never really reliable, since the first command could be to
a plugin for which we simply pass through the JSON (though, carefully
appending the expected '\n\n' if not already there).
Reported-by: @laanwj
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
That was changed to start the response object, which broke the openingd
code once we merged.
Of course, I should have *renamed it* when I changed the semantic!
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Compile broke because we were using low-level JSON primitives here
(which, incidentally, would produce bad JSON now, since we can't just
put a raw string inside an object!).
Use json_add_string, which also has the benefit of escaping JSON
for us.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Big wiring re-org for funding-continue
In openingd, we move the 'persistent' state (their basepoints,
pubkey, and the minimum_depth requirement for the opening tx) into
the state object. We also look to keep code-reuse between
'continue' and normal 'fundchannel' as high as possible. Both
of these call the same 'fundchannel_reply' at the end.
In opening_control.c, we remap fundchannel_reply such that it is
now aware of the difference between an external/internally funded
channel open. It's the same return path, with the difference that
one finishes making and broadcasting the funding transaction; the
other is skips this.
Add an RPC method (not working at the moment) called
`fundchannel_continue` that takes as its parameters a
node_id and a txid for a transaction (that ostensibly has an output
for a channel)
Some channels won't be opened with a wtx struct, so keep
the total funding amount separate from it so we can
show some stats for listpeers.
Note that we're going to need to update/confirm this once
the transaction gets confirmed.
For the `fundchannel_cancel` we're going to want
to 'successfully' fail a funding channel operation. This allows
us to report it a failure back as an RPC success, instead of
automatically failing the RPC request.
We're going to need this for P2WSH scripts. pull it out into
a common file plus adopt the sanity checks so that it will allow for
either P2WSH or P2WPKH (previously only encoded P2WPKH scripts)
This is an old bug, where a plugin can get called while we're shutting
down (and have freed plugins), but it's triggered more reliably by the
new warning notification hook.
For good measure, we also make freeing a plugin self-delete.
Valgrind error file: valgrind-errors.16763
==16886== Invalid read of size 8
==16886== at 0x422919: plugins_notify (plugin.c:1096)
==16886== by 0x413919: notify_warning (notification.c:61)
==16886== by 0x412BDE: logv (log.c:251)
==16886== by 0x412A98: log_ (log.c:311)
==16886== by 0x4044BE: bcli_finished (bitcoind.c:178)
==16886== by 0x459480: destroy_conn (poll.c:244)
==16886== by 0x459499: destroy_conn_close_fd (poll.c:250)
==16886== by 0x4619E1: notify (tal.c:235)
==16886== by 0x461A7E: del_tree (tal.c:397)
==16886== by 0x461AB5: del_tree (tal.c:407)
==16886== by 0x461AB5: del_tree (tal.c:407)
==16886== by 0x461AB5: del_tree (tal.c:407)
==16886== Address 0x634a578 is 40 bytes inside a block of size 352 free'd
==16886== at 0x4C2EDEB: free (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==16886== by 0x461AFD: del_tree (tal.c:416)
==16886== by 0x461FB7: tal_free (tal.c:481)
==16886== by 0x411E0A: main (lightningd.c:841)
==16886== Block was alloc'd at
==16886== at 0x4C2DB8F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==16886== by 0x4617CE: allocate (tal.c:245)
==16886== by 0x461E4C: tal_alloc_ (tal.c:423)
==16886== by 0x42255E: plugins_new (plugin.c:106)
==16886== by 0x41133D: new_lightningd (lightningd.c:218)
==16886== by 0x411AD4: main (lightningd.c:649)
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This is a painpoint with testing, that there's a noticable delay between
"Shutting down" from lightning-cli and being able to restart lightningd.
This fixes that by creating a canned response for this case, which is
simply written out immediately before exit. At this point, the pidfile
has been deleted, the sockets have been closed, and the database
has been closed.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Move it closer to ccan/json_out, in preparation for using that as a
replacement.
In particular:
1. Add a 'quote' field in json_add_member.
2. json_add_member now always escapes if 'quote' is true.
3. json_member_direct is exposed to allow avoiding of escaping.
4. json_add_hex can use this, so no longer needs to be in json_stream.c.
5. We don't make JSON manually, but always use helpers.
6. We now flush the stream (wake reader) only when we close it, or mark
command as pending.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
"result" should always be an object (so that we can add new fields),
so make that implicit in json_stream_success.
This makes our primitives well-formed: we previously used NULL as our
fieldname when calling the first json_object_start, which is a hack
since we're actually in an object and the fieldname is 'result' (which
was already written by json_object_start).
There were only two cases which didn't do this:
1. dev-memdump returned an array. No API guarantees on this.
2. shutdown returned a string.
I temporarily made shutdown return an empty object, which shouldn't
break anything, but I want to fix that later anyway.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
These are generalized from our internal implementations.
The main difference is that 'struct json_escaped' is now 'struct
json_escape', so we replace that immediately.
The difference between lightningd's json-writing ringbuffer and the
more generic ccan/json_out is that the latter has a better API and
handles escaping transparently if something slips through (though
it does offer direct accessors so you can mess things up yourself!).
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This new parameter takes a list of outpoints (as txid:vout) and fund a channel from the corresponding utxos.
Example : fundchannel <id> 10000 normal 1 [10767f0db0e568127fffd7f70a154d4599f42d62babf63230a7c3378bfce3cb0:0, c9e040e0b5fc8c59d5e7834108fbc5583001f414dd83faf0a05cff9d1a92d32c:0]
This means there's now a semantic difference between the default `fromid`
and setting `fromid` explicitly to our own node_id. In the default case,
it means we don't charge ourselves fees on the route.
This means we can spend the full channel balance.
We still want to consider the pricing of local channels, however:
there's a *reason* to discount one over another, and that is to bias
things. So we add the first-hop fee to the *risk* value instead.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Take into account the fee we'd have to pay if we're the funder, and
also drop to 0 if the amount is less than the smallest HTLC the peer
will accept.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This was incorrectly handled before, hence the wrapper which checks
correctness of the arguments.
Signed-off-by: Christian Decker <decker.christian@gmail.com>
This is important for things we automatically watched because it spends a
watch txo, but only onchaind knows the details about what the TX really is and
how it should be handled.
Signed-off-by: Christian Decker <decker.christian@gmail.com>
Rusty Russell
Christian Decker
lisa neigut
darosior