We were restarting the with the nodes before, which was causing some
port contention. This is more natural since `bitcoind` will take care
of terminating all proxies it returned.
Signed-off-by: Christian Decker <decker.christian@gmail.com>
Logging an empty line (without newline character) would raise an
Exception due to out of bounds check.
Signed-off-by: Christian Decker <decker.christian@gmail.com>
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>
My manpage viewer did not know what to do with curly braces,
so I switched them to quotes and it works fine.
Signed-off-by: Mark Beckwith <wythe@intrig.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>
I upgraded my node with --disable-compat, and a heap of channels closed like:
CHANNELD_NORMAL:We disagree on short_channel_ids: I have 557653x0x1351, you say 557653x2373x1",
This is because the scids are strings in the databases, and it failed to parse
them properly.
Now we'll not start if that happens.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Internally libplugin turns ' into ", which causes these messages to produce
bad JSON.
The real fix is to remove the '->" convenience substitution and port the
JSON creation APIs into common/ from lightningd/
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Spurious errors were occuring around checking the provided
current commitment point from the peer on reconnect when
option_data_loss_protect is enabled. The problem was that
we were using an inaccurate measure to screen for which
commitment point to compare the peer's provided one to.
This fixes the problem with screening, plus makes our
data_loss test a teensy bit more robust.
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>
jsmn would accept invalid JSON objects. This is bad because it would
set ->size incorrectly: we expect to have at least size * 2 tokens (in
pairs). We want to rely on ->size, but this would create an exploitable
buffer overflow!
Fortunately, this is fixed upstream, so we add a test and upgrade to v1.0.0.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Wasn't using valid JSON, but worked anyway. This is actually OK
because we don't rely on tok->size, but we want to, so another fix
coming.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
The external/jsmn/README.md only says:
int size; // Number of child (nested) tokens
But it only counts *direct* children, or *direct* members for an object.
This test verifies this (the bug proved to be elsewhere: see next patch!).
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Avoid the unnecessary extra var, and don't use "capacity" since
that usually refers to static capacity.
Reported-by: @cdecker
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Had a couple of tests randomly fail because a valgrind error file was
not empty. It contained:
lightning_channeld: Writing out status 65520: Broken pipe
This shouldn't happen, but the simplest workaround is not to print
that (useless) error.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
So add a new 'strategy' field. This makes it clearer what is going
on, currently one of:
* "Initial attempt"
* "Excluded channel <scid>"
* "Removed route hint"
* "Excluded expensive channel <scid>"
* "Excluded delaying channel <scid>"
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We sanitize the routes: firstly, we assume appending so eliminate the
first hop if the route points straight to us. Secondly, eliminate empty
hints. Thirdly, trim overlong hints.
Then we just use the first route hint.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We use the 'exclude' option to getroute for successive attempts. This
is more robust than having gossipd disable for some limited time.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
I wrote this sync first, then rewrote async, then developed libplugin.
But committing all that just wastes reviewer time, so I present it as
if it was always asnc and using the library helper.
Currently the command it registers is 'pay2', but when it's complete
we'll remove the internal 'pay' and rename it. This does a single
'getroute/sendpay' call. No retries, no options.
Shockingly, this by itself is almost sufficient to pass our current test
suite with `pay`->`pay2`.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Christian Decker
arowser
Mark Beckwith
Rusty Russell
lisa neigut