Final step for the `peer_connected` hook, we parse the result and act
accordingly. Currently we just close the underlying connection, but we
may want to clean up peers that did not end up with a channel.
Signed-off-by: Christian Decker <decker.christian@gmail.com>
The format is very similar to the one for `listpeers` except we only
list a single channel, and we list the actual netaddr that connected
instead of all known from gossip.
Signed-off-by: Christian Decker <decker.christian@gmail.com>
This used to be inline, but we want to pass channels to hooks as well,
so we just extract this into its own function.
Signed-off-by: Christian Decker <decker.christian@gmail.com>
This hook is used to let a plugin decide whether we should continue
with the connection normally, or whether we should be dropping the
connection. Possible use-cases include policy enforcement (dropping
connections based on previous interactions), draining a node by
allowing only peers with an active channel to reconnect, or
temporarily preventing a channel from making progress.
Signed-off-by: Christian Decker <decker.christian@gmail.com>
I got a spurious failure because the final node gave a CLTV error and
so it decided to use a different channel. It should probably handle
this corner case better, but meanwhile make the test robust.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This patch will properly set fee_per_satoshi to _unsigned_ integer,
as support for negative fees was removed from overall design.
This change does not break any tests, so I assume its
better this way.
We need to still accept it when parsing the database, but this flag
should allow upgrade testing for devs building on top
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We store it in a strmap. This means we call the jsonrpc handler earlier,
so all callers need to call param() before they do anything else; only
json_listaddrs and json_help needed fixing.
Plugins still use '[usage]' for now.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Next patch will call commands to get usage inside jsonrpc_new(): to do
this it will need access to ld->jsonrpc, so we can't use the current
pattern.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This fixes a bug with a plugin duplicating an existing name
where we'd crash, too.
This doesn't work for builtins, which aren't tal objects, so
create a separate path for them.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We used a u16, and a 1000 multiplier, which meant we wrapped at
riskfactor 66. We also never undid the multiplier, so we ended up
applying 1000x the riskfactor they specified.
This changes us to pass the riskfactor with a 1M multiplier. The next
patch changes the definition of riskfactor to be more useful.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
- result fundchannel command now depends on successful or failed broadcast of the funding tx
- failure returns error code FUNDING_BROADCAST_FAIL
- don't fail the channel when broadcast failed, but keep in CHANNELD_AWAITING_LOCKIN
- after fixing the initial broadcast failure, the user could manually rebroadcast the tx and
keep the channel
openingd/opening_funder_finished:
- broadcast_tx callback function now handles both success and failure
jsonrpc: added error code FUNDING_BROADCAST_FAIL
manpage: added error code returned by fundchannel command
This makes the user more aware of broadcast failure, so it hopefully doesn't
try to broadcast new tx's that depend on its change_outputs. Some users have reported (see
issue #2171) a whole sequence of fundings failing, because each funding was using the change
output of the previous one, which would not confirm.
The use of `json_tok_full_len` and `json_tok_full` in addition to
single quotes will result in double quoting, which is really weird. I
opted to single quoting using `'` instead which does not need to be
escaped.
Signed-off-by: Christian Decker <decker.christian@gmail.com>
List the final one instead; if there's an error from the node it
may actually make sense to blame that channel (ie. previous node
did something wrong).
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We no longer need a 'sendpay_result' structure, we can pass
appropriate parameter directly now they're simple calls.
Every waitsendpay command ends in tell_waiters_failed or
tell_waiters_success, which call sendpay_success or sendpay_fail on
all matching waiters. These all return 'struct command_result *'.
In cases where the result is immediately known, we call
sendpay_success/sendpay_fail directly for the command.
This also adds a helpful 'failcodename' field to the JSON output.
[ This was four separate cleanup patches, but that contained much
redundancy and was even worse to review ]
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
With only one caller, we don't need a callback pointer any more; we can simply
call the function.
This required some code shuffling, and I changed the callback function
arguments to be in a more natural order, now they're not used as
callbacks.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Now we don't have a second caller for these routines, we can move
them back into pay.c and make the functions static.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
As a general rule, lightningd shouldn't parse user packets. We move the
parsing into gossipd, and have it respond only to permanent failures.
Note that we should *not* unconditionally remove a channel on
WIRE_INVALID_ONION_HMAC, as this can be triggered (and we do!) by
feeding sendpay a route with an incorrect pubkey.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We had a bug 0ba547ee10 caused by
short_channel_id overflow. If we'd caught this, we'd have terminated
the peer instead of crashing, so add appropriate checks.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Valgrind seems to be slowing the pay-plugin down enough for the 10
seconds timeout to get triggered on a semi-regular basis.
Reported-by: Rusty Russell <@rustyrussell>
Signed-off-by: Christian Decker <decker.christian@gmail.com>
Christian points out that we can iterate by ->size rather than calling
json_next() to find the end (which traverses the entire object!).
Now ->size is reliable (since previous patch), this is OK.
Reported-by: @cdecker
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We therefore keep a reference to the DB and will wrap and unwrap when
a hook returns.
Notice that this might cause behavior changes when moving logic into a
hook callback, since the continuation runs in a different transaction
than the event that triggered the hook in the first place. Should not
matter too much, since we don't use DB rollbacks at the moment, but
it's something to keep in mind.
Signed-off-by: Christian Decker <decker.christian@gmail.com>
This ties all the things together, using the serializer to transform
the payload into a valid `jsonrpc_request`, sending it to the plugin,
and then using the deserializer on the way back before calling the
hook callback with the appropriate information.
Notice that the serializer and deserializer is skipped if we don't
have a plugin that registered for this hook.
Signed-off-by: Christian Decker <decker.christian@gmail.com>
plugin_request_new did nothing special aside from registering the
request ID with the dispatch code. This duty has now been moved to
plugin_request_send instead, which is also exposed so we can use that
code in plugin_hook.
Signed-off-by: Christian Decker <decker.christian@gmail.com>
This is the first use of the `hooks` autodata field, and it required a
dummy element in order for the section not to be dropped, it'll be
removed once we have actual hooks.
There is very little that is plugin specific in the jsonrpc_request so
this just extracts the common parts so we can reuse them outside of
the plugin compilation unit as well.
None of the existing callbacks was making use of it and we will be
exposing the method callback interface to outside compilation unit
where the struct definition is not visible. So just remove it.
Signed-off-by: Christian Decker <decker.christian@gmail.com>
I might have gone a bit overboard with the type-checking, but
typesafe_cb_cast is quite nice to use, so why not. The macro to
register a new hook encapsulates the entire flow from param
serialization, to dispatch, parsing and callback dispatch in one
bundle. I was tempted to have the callback outside of the
registration, but it's unlikely that we'll have two calls to the same
hook with different callbacks.
Signed-off-by: Christian Decker <decker.christian@gmail.com>
Currently only used by gossipd for channel elimination.
Also print them in canonical form (/[01]), so tests need to be
changed.
Suggested-by: @cdecker
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We use the PAY error code here, but it's appropriate (otherwise the
pay command simply has to substitute it, which seems silly).
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
seed isn't very useful at this level: I've left it in routing.c
because it might be useful for detailed testing. Pretty sure it's unused,
so I simply removed it.
The fuzzpercent is documented to default at 5%, but actually was 75%.
Fix that too.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Christian and I both unwittingly used it in form:
*tal_arr_expand(&x) = tal(x, ...)
Since '=' isn't a sequence point, the compiler can (and does!) cache
the value of x, handing it to tal *after* tal_arr_expand() moves it
due to tal_resize().
The new version is somewhat less convenient to use, but doesn't have
this problem, since the assignment is always evaluated after the
resize.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This is based on Christian's change, but removes all trace of the old codes.
I've proposed another spec change which removes this code altogether:
https://github.com/lightningnetwork/lightning-rfc/pull/544
Signed-off-by: Christian Decker <decker.christian@gmail.com>
Reported-by: Rusty Russell <@rustyrussell>
This is mainly just copying over the copy-editing from the
lightning-rfc repository.
[ Split to just perform changes prior to the UNKNOWN_PAYMENT_HASH change --RR ]
Signed-off-by: Christian Decker <decker.christian@gmail.com>
Reported-by: Rusty Russell <@rustyrussell>
Since we are planning to release a bug fix release, and the plugin
subsystem is not yet complete, it is better to make plugin support
opt-in while we continue testing.
Signed-off-by: Christian Decker <decker.christian@gmail.com>
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Fortunately, we can calculate the sha256 ourselves, so the
outgoing channeld doesn't need to tell us.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
The node which sent the error is doing so because the following
one sent WIRE_UPDATE_FAIL_MALFORMED_HTLC.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This covers all the cases where an onion can be malformed; this means
we know in advance that it's bad. That allows us to distinguish two
cases: where lightningd rejects the onion as bad, and where the next
peer rejects the next onion as bad. Both of those (will) set failcode
to one of the BADONION values.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We currently use 'all-zeroes' as 'unknown', but NULL is more natural
even if we have to send it as all-zeroes over the wire due to
expressiveness limitations in our generation code.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
The processes that were used to test the subdaemon versions were not
reaped correctly keeping some resources bound.
Signed-off-by: Christian Decker <decker.christian@gmail.com>
Error on gcc 7.3.0:
lightningd/peer_control.c: In function ‘json_close’:
lightningd/peer_control.c:955:3: error: ‘channel’ may be used uninitialized in
this function [-Werror=maybe-uninitialized]
channel_set_state(channel,
^~~~~~~~~~~~~~~~~~~~~~~~~~
channel->state, CHANNELD_SHUTTING_DOWN);
Signed-off-by: William Casarin <jb55@jb55.com>
Switch to write_all instead
Error on gcc 7.3.0:
lightningd/lightningd.c: In function ‘on_sigterm’:
lightningd/lightningd.c:587:9: error: ignoring return value of ‘write’, declared
with attribute warn_unused_result [-Werror=unused-result]
write(STDERR_FILENO, msg, strlen(msg));
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Signed-off-by: William Casarin <jb55@jb55.com>
This used to be request-specific, but we now want to send
notifications and requests. As a drive-by we also clarify the
ownership of the json_stream instance that is being sent.
Signed-off-by: Christian Decker <decker.christian@gmail.com>
Will be used in the next commit to fan out notifications to multiple
subscribing plugins. We can't just use `tal_dup` from outside since
the definition is hidden outside the compilation unit.
Signed-off-by: Christian Decker <decker.christian@gmail.com>
Obviously the Facebook relationship status joke was a bit subtle, but I've
continued it anyway because I'm especially susceptible to Dad jokes.
Suggested-by: @niftynei
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
memdump iterates through the various daemons asking them to check for
leaks.
We currently call openingds (there might be none), channelds (there
might be none), then hsmd synchronously (the other daemons). If hsmd
reports a leak, we'll fail the dev-memleak command immediately.
Change the order to call connectd first; that's always async, so we
can happily mark the command still pending.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This causes a compiler warning if we don't do something with the
result (hopefully return immediately!).
We use was_pending() to ignore the result in the case where we
complete a command in a callback (thus really do want to ignore
the result).
This actually fixes one bug: we didn't return after command_fail
in json_getroute with a bad seed value.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Usually, this means they return 'command_param_failed()' if param()
fails, and changing 'command_success(); return;' to 'return
command_success()'.
Occasionally, it's more complex: there's a command_its_complicated()
for the case where we can't exactly determine what the status is,
but it should be considered a last resort.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Handers of a specific form are both designed to be used as callbacks
for param(), and also dispose of the command if something goes wrong.
Make them return the 'struct command_result *' from command_failed(),
or NULL.
Renaming them just makes sense: json_tok_XXX is used for non-command-freeing
parsers too.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
These routines free the 'struct command': a common coding error is not
to return immediately.
To catch this, we make them return a non-NULL 'struct command_result
*', and we're going to make the command handlers return the same (to
encourage 'return command_fail(...)'-style usage).
We also provide two sources for external use:
1. command_param_failed() when param() fails.
2. command_its_complicated() for some complex cases.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
These are only supposed to be used when you want the token contents including
surrounding "". We should use this when reporting errors, but usually
we just want to access the tok members directly.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Live connections can confuse us; this happens a lot more when we're
running complex plugins, since they make JSONRPC connections while we're
running our tests.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
