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>
