We don't compile with NDEBUG defined, but if we did, this code would
vanish. I did a quick audit, inspired by @ZmnSCPxj.
I actually hacked up something to compile with NDEBUG (many unused vars
resulted, and of course unit tests are allowed to rely on assert()), and
after this the testsuite still passes.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
the tests are not possible only by having tests/requirements.txt .
Running the whole testsuite also runs contrib/pyln-proto/tests/test_invoice.py
which needs requiremnets of pyln-proto and so on.
Also requirement coincurve requires libsecp256k1-dev headers.
As most developers need all requiremtents and we should tell them.
Update to v13 as v12 started to raise undefined symbol exceptions agains latest
libsecp256k1.
Note: any version of `pip install coincurve` fails if no libsecp256k1 headers
are installed, should we point this out somewhere/somehow?
Changelog-None
I reproduced this by putting a sleep(60) in the pay plugin, then
'lightning-cli pay', 'lightning-cli plugin stop pay' and then ^C
the `lightning-cli pay`:
2020-02-14T00:33:11.217Z INFO plugin-pay: Killing plugin: pay stopped by lightningd via RPC
2020-02-14T00:33:15.250Z DEBUG lightningd: Still waiting for initial block download
==5157== Invalid read of size 8
==5157== at 0x12A29C: destroy_jcon (jsonrpc.c:149)
==5157== by 0x1C6F2A: notify (tal.c:235)
==5157== by 0x1C7441: del_tree (tal.c:397)
==5157== by 0x1C7493: del_tree (tal.c:407)
==5157== by 0x1C77DD: tal_free (tal.c:481)
==5157== by 0x1B7380: io_close (io.c:450)
==5157== by 0x1B71A7: do_plan (io.c:401)
==5157== by 0x1B7214: io_ready (io.c:417)
==5157== by 0x1B94AC: io_loop (poll.c:445)
==5157== by 0x1291C9: io_loop_with_timers (io_loop_with_timers.c:24)
==5157== by 0x12EC7E: main (lightningd.c:928)
==5157== Address 0x4ebab98 is 40 bytes inside a block of size 88 free'd
==5157== at 0x483BA3F: free (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==5157== by 0x1C750F: del_tree (tal.c:416)
==5157== by 0x1C7493: del_tree (tal.c:407)
==5157== by 0x1C77DD: tal_free (tal.c:481)
==5157== by 0x153856: clear_plugin (plugin_control.c:209)
==5157== by 0x1538FF: plugin_dynamic_stop (plugin_control.c:225)
==5157== by 0x153C51: json_plugin_control (plugin_control.c:295)
==5157== by 0x12B4EC: command_exec (jsonrpc.c:588)
==5157== by 0x12B8AB: rpc_command_hook_callback (jsonrpc.c:679)
==5157== by 0x154575: plugin_hook_call_ (plugin_hook.c:170)
==5157== by 0x12BCD3: plugin_hook_call_rpc_command (jsonrpc.c:756)
==5157== by 0x12BD04: call_rpc_command_hook (jsonrpc.c:764)
==5157== Block was alloc'd at
==5157== at 0x483A7F3: malloc (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==5157== by 0x1C6F98: allocate (tal.c:245)
==5157== by 0x1C7559: tal_alloc_ (tal.c:423)
==5157== by 0x15135A: plugin_rpcmethod_add (plugin.c:706)
==5157== by 0x151600: plugin_rpcmethods_add (plugin.c:756)
==5157== by 0x151BDD: plugin_parse_getmanifest_response (plugin.c:893)
==5157== by 0x151C9C: plugin_manifest_cb (plugin.c:915)
==5157== by 0x14FFB9: plugin_response_handle (plugin.c:258)
==5157== by 0x150165: plugin_read_json_one (plugin.c:356)
==5157== by 0x1502BC: plugin_read_json (plugin.c:388)
==5157== by 0x1B65ED: next_plan (io.c:59)
==5157== by 0x1B71D2: do_plan (io.c:407)
Fixes: #3509
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
If the peer is not connected, or other error which means we don't
actually create an outgoing HTLC, we don't record the
short_channel_id. This is unhelpful!
Pass the scid down to the wallet code, and explicitly hand the
scid and amount down to the notification code rather than handing it
the htlc_out (which it doesn't need).
Changelog-Changed: JSON API: `listforwards` now shows `out_channel` even if we couldn't forward.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Spark does this, for example:
{"method":"pay","params":["lnbc..."],"id":22}
Which doesn't have a jsonrpc field. The result is that the command
doesn't terminate, there is nothing in the logs, stderr contains
"pay: JSON-RPC message does not contain "jsonrpc" field", and
from then on "Unknown command 'pay'".
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Normalized lines; split some where we've deprecated something (needs a
line each in Deprecated section).
Also, removed unused [Unreleased] footnote in favor of [0.8.0] footnote.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
1. GH_TOKEN is probably required to run the changelog script.
2. Note the footnotes need updating; we no longer have [Unreleased].
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
For bitcoind_fail_first:
We only ever send `getblock` if we got a successful block hash from
`getblockhash`, and if we can't get the block in that case it means
our Bitcoin backend is faulty and we shouldnt continue.
So, mock `getblockhash` instead, which is authorized to spuriously fail.
For both bitcoind_fail_first and bitcoind_failure:
Adapt the logs.
This avoids the getblockhash+getblock, and more importantly that was the
last functionality making use of bitcoind_getrawblock() and bitcoin_getblockhash(),
so we can also get rid of them.
This adds `getchaininfo` and `getrawblockbyheight` handling lightningd-side,
and use them in setup_topology().
We then remove legacy bitcoind_getblockcount() (we already get the count in
`getchaininfo`), bitcoind_getblockchaininfo() (it was only used in setup_topology()),
and wait_for_bitcoind() (this was specific to bitcoin-core and we assume our Bitcoin
backend to be functional if the plugin responds to `init`).
We are going to initialize a plugin before its creation, so log as
UNUSUAL instead.
Also, `pay` and `fundchannel` inits are using rpc_delve(), so we need to
io_new_conn() (which sets the socket as non blocking) after calling the
plugin's init.
This is also taken and adapted from lightningd/bitcoind.
The call to 'getblockchaininfo' is replaced by 'echo' as we don't
make use of the result and the former can sometimes be slow (e.g. on
IBD).
Most is taken from lightningd/bitcoind and adapted. This currently
exposes 5 commands:
- `getchaininfo`, currently called at startup to check the network and
whether we are on IBD.
- `getrawblockbyheight`, which basically does the `getblockhash` +
`getblock` trick.
- `getfeerate`
- `sendrawtransaction`
- `getutxout`, used to gather infos about an output and currently used by
`getfilteredblock` in `lightningd/bitcoind`.
We don't take the callback result into account, so it can better be void.
Having a general callback parameter is handy, because for bcli we want
to pass it the struct bcli.
Add new check if we're funder trying to add HTLC, keeping us
with enough extra funds to pay for another HTLC the peer might add.
We also need to adjust the spendable_msat calculation, and update
various tests which try to unbalance channels. We eliminate
the now-redundant test_channel_drainage entirely.
Changelog-Fixed: Corner case where channel could become unusable (https://github.com/lightningnetwork/lightning-rfc/issues/728)
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Make the `htlc_accepted` hook the first chained hook in our repertoire. The
plugins are called one after the other in order until we have no more plugins
or the HTLC was handled by one of the plugins. If no plugins handles the HTLC
we continue to handle it internally like always.
Handling in this case means the plugin returns either `{"result": "resolve",
...}` or `{"result": "fail", ...}`.
Changelog-Changed: plugin: Multiple plugins can now register for the htlc_accepted hook.
This used to be necessary because we allocated the `plugin_hook_request` off
of the plugin instance (only tal allocated object we could grab at that
time. Now the plugin was replaced by a list, which itself is tal-allocated,
making that workaround pointless, or even wrong once we have multiple plugins
registering for that hook.
ZmnSCPxj jxPCSnmZ
darosior
Rusty Russell
Michael Schmoock
Christian Decker