That's more convenient for most callers, which don't need a fmt.
Fixed-by: Darosior <darosior@protonmail.com>
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This is what I expected from plugin_kill, and now all the callers do the
equivalent anywat, it's easy.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Instead of calling plugin_kill() and returning, have them
uniformly return an error string or NULL, and have the top
level (plugin_read_json) do the plugin_kill() call.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This means we now clean up options in startup plugins (that was only
done by dynamic code!), and now they both share the 60 second timeout
instead of 20 seconds for dynamic.
For the dynamic case though, it's 60 seconds to both complete
getmanifest and init, which seems fair.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This will allow the dynamic starting code to use them too.
Also lets us move dev_debug_subprocess under #if DEVELOPER.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This will let us unify the startup and runtime-started infrastructure.
Note that there are two kinds of notifications:
1. Starting a single plugin (i.e. `plugin start`)
2. Starting multiple plugins (i.e. `plugin rescan` or `plugin startdir`).
In the latter case, we want the command to complete only once *all*
the plugins are dead/finished.
We also call plugin_kill() in all cases, and correctly return afterwards
(it matters once we use the same paths for dynamic plugins, which don't
cause a fatal error if they don't startup).
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Now we know whether the command completed or not, we can correctly
call command_still_pending() if it didn't complete.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
The symptom (under heavy load and valgrind) in test_plugin_command:
lightningd: common/json_stream.c:237: json_stream_output_: Assertion `!js->reader' failed.
This is because we try to call `getmanifest` again on `pay` which has not yet
responded to init.
The minimal fix for this is to keep proper state, so we can tell the
difference between "not yet called getmanifest" and "not yet finished
init".
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
I noticed the following in logs for tests/test_connection.py::test_feerate_stress:
```
DEBUG 022d223620a359a47ff7f7ac447c85c46c923da53389221a0054c11c1e3ca31d59-chan#1: Failing HTLC 18446744073709551615 due to peer death
DEBUG 022d223620a359a47ff7f7ac447c85c46c923da53389221a0054c11c1e3ca31d59-chan#1: local_routing_failure: 8194 (WIRE_TEMPORARY_NODE_FAILURE)
```
This is because it reports the (transient) node_failure error, because
our channel_failure message is incomplete. Fix this wart up.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Previously we've used the term 'funder' to refer to the peer
paying the fees for a transaction; v2 of openchannel will make
this no longer true. Instead we rename this to 'opener', or the
peer sending the 'open_channel' message, since this will be universally
true in a dual-funding world.
This in addition removes the init fixed timeout hack.
Changelog-fixed: We now *always* die if our Bitcoin backend failed unexpectedly.
Signed-off-by: Antoine Poinsot <darosior@protonmail.com>
Commit 9aedb0c61f changed this from allocating off `c` to allocating
off NULL, knowing that it's tal_steal() in the callback. But before
that, it can be detected as a mem leak:
```
@pytest.fixture
def teardown_checks(request):
"""A simple fixture to collect errors during teardown.
We need to collect the errors and raise them as the very last step in the
fixture tree, otherwise some fixtures may not be cleaned up
correctly. Require this fixture in all other fixtures that need to either
cleanup before reporting an error or want to add an error that is to be
reported.
"""
errors = TeardownErrors()
yield errors
if errors.has_errors():
# Format a nice list of everything that went wrong and raise an exception
request.node.has_errors = True
> raise ValueError(str(errors))
E ValueError:
E Node errors:
E Global errors:
E - Node /tmp/ltests-iz9y1chb/test_hsmtool_secret_decryption_1/lightning-1/ has memory leaks: [
E {
E "backtrace": [
E "ccan/ccan/tal/tal.c:442 (tal_alloc_)",
E "lightningd/jsonrpc.c:848 (parse_request)",
E "lightningd/jsonrpc.c:941 (read_json)",
E "ccan/ccan/io/io.c:59 (next_plan)",
E "ccan/ccan/io/io.c:407 (do_plan)",
E avis/build/ElementsProject/lightning/lightningd/../plugins/pay
```
Reported-by: @niftynei
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
The plugin can basically return whatever it thinks the preimage is, but we
weren't handling the case in which it doesn't actually match the hash. If it
doesn't match now we just return an error claiming we don't have any matching
invoice.
We use the new function `plugins_free` to define the correct deallocation
order on shutdown, since under normal operation the allocation tree is
organized to allow plugins to terminate and automatically free all dependent
resources. During shutdown the deallocation order is under-defined since
siblings may get freed in any order, but we implicitly rely on them staying
around.
One is called on every plugin return, and tells us whether to continue;
the other is only called if every plugin says ok.
This works for things like payload replacement, where we need to process
the results from each plugin, not just the final one!
We should probably turn everything into a chained callback next
release.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
They callback must take ownership of the payload (almost all do, but
now it's explicit).
And since the payload and cb_arg arguments to plugin_hook_call_() are
always identical, make them a single parameter.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We have several of these, and they're not always called obvious things like
"delete" or "free". `STEALS` provides a strong hint here.
I only added it to a couple I knew about off the top of my head.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This makes testing easier, and makes sense: lightningd might not
*know* about other connected channels, depending on gossip, but if the
user specifies it we should obey it.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Changed: JSON: `invoice` `exposeprivatechannels` now includes explicitly named channels even if they seem like dead-ends.
This is what actually lets us pay blinded invoices.
Unfortunately, our internal logic assumes every hop in a path has a
next `short_channel_id`, so we have to use a dummy. This is
sufficient for testing, however.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This will be used when we want to specify these in a route. But for now, they
only alter gossipd, which always sets them to NULL.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Note that it's channeld which calculates the shared secret, too. This
minimizes the work that lightningd has to do, at cost of passing this
through.
We also don't yet save the blinding field(s) to the database.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This requires us to call ecdh() in the corner case where the blinding seed
is in the TLV itself (which is the case for the start of a blinded route).
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We now track all pending RPC passthrough calls, and terminate them with an
error if the plugin dies.
Changelog-Fixed: JSON-RPC: Pending RPC method calls are now terminated if the handling plugin exits prematurely.
Use `LC_ALL=C sort` instead of `sort` so that mocks get sorted in
the same way on all developers' environments.
Re-record the result of `make update-mocks`.
Changelog-None
This happened on my testnet node because I've been failing to reconnect to
a node which created a channel and never exchanged announcement sigs.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
common/onion is going to need to use this for the case where it finds a blinding
seed inside the TLV. But how it does ecdh is daemon-specific.
We already had this problem for devtools/gossipwith, which supplied a
special hsm_do_ecdh(). This just makes it more general.
So we create a generic ecdh() interface, with a specific implementation
which subdaemons and lightningd can use.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We currently abuse the added_htlc and failed_htlc messages to tell channeld
about existing htlcs when it restarts. It's clearer to have an explicit
'existing_htlc' type which contains all the information for this case.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
It's almost always "their_features" and "our_features" respectively, so
make those names clear.
Suggested-by: @cdecker
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Note that now we check capacity once we've figured out which peer, which
broke a test (we returned "unknown peer" instead of "capacity exceeded"),
so we rework that too.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Rusty Russell
rbndg
Sebastian Falbesoner
lisa neigut
Antoine Poinsot
Christian Decker
Vasil Dimov