The current code sends hsmstatus_client_bad_request via the req fd;
this won't work, since lightningd uses that synchronously and only
expects a reply to its commands. So send it via status_conn.
We also enhance hsmstatus_client_bad_request to include details, and
create convenience functions for it. Our previous handling was ad-hoc;
we sometimes just closed on the client without telling lightningd,
and sometimes we didn't tell lightningd *which* client was broken.
Also make every handler the exact same prototype, so they now use the
exact same patterns (hsmd *only* handles requests, makes replies).
I tested this manually by corrupting a request to hsmd.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We currently just ignore them. This is one reason the hsm (in some places)
explicitly calls log_broken so we get some idea.
This was the only subdaemon which had a NULL msgcb and msgname, so eliminate
those checks in subd.c.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
The `json_tok_percentage` parser is used for the `fuzzpercent` in `getroute` and
`maxfeepercent` in `pay`. In both cases it seems reasonable to allow values
larger than 100%. This has bitten users in the past when they transferred single
satoshis to things like satoshis.place over a route longer than 2 hops.
With the previous patch, we could still get stuck behind a low-prio
request. Generalize it into separate queues, and allow more than one
request in parallel.
Worth noting that the test time for `VALGRIND=0 pytest -vx tests/ -n 10`
doesn't change measurably.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
fiatjaf has a cheap VPS, connecting remotely to his home bitcoind node.
fiatjaf's latency on bitcoin-cli getblock is between 10 and 37 seconds.
fiatjaf's c-lightning node is getting one block per hour.
fiatjaf is sad.
We single-file our bitcoind requests, because bitcoind has a limited
thread pool and it *fails* rather than queueing if you upset it. We
probably be fine using separate queues for each command type, but simply
allowing some requests to cut in line should prove my theory that we're
getting stuck behind gossip verification requests.
fiatjaf now gets one block per 2 minutes.
fiatjaf is less sad.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Everything depends on common headers etc, and the HSM_CLIENT_HEADERS was removed
quite a while ago.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We would never complete further ping commands if we had < responses
than pings. Oops.
Fixes: #1928
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We want to try it before --daemon, in case we error, but we don't know
the pid yet, so we split into 'lock' and 'write'.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
If we run two daemons on the same directory we'd be getting the failure from
trying to listen to the same file before we'd hit the pid-file error, which was
causing confusion.
The first argument of 'ping' was documented as 'peerid', however
internally it is expected to be just 'id'.
To avoid breaking the API, opt to fix the documentation.
This was found because it means we have a non-zero feerate without
filling in the history of that feerate:
==15895== Conditional jump or move depends on uninitialised value(s)
==15895== at 0x408699: feerate_max (chaintopology.c:828)
==15895== by 0x41BE49: peer_start_openingd (opening_control.c:733)
==15895== by 0x425FE9: peer_connected (peer_control.c:515)
==15895== by 0x40CB8F: connectd_msg (connect_control.c:304)
==15895== by 0x42DB4E: sd_msg_read (subd.c:475)
==15895== by 0x42D499: read_fds (subd.c:302)
==15895== by 0x46EB18: next_plan (io.c:59)
==15895== by 0x46F5E9: do_plan (io.c:387)
==15895== by 0x46F627: io_ready (io.c:397)
==15895== by 0x471187: io_loop (poll.c:310)
==15895== by 0x41683D: main (lightningd.c:732)
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Documentation changes:
1. Lots of extra detail suggested by @renepickhardt.
2. typo fixes from @practicalswift.
3. A section on 'const' usage.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Code changes:
1. Expose daemon_poll() so lightningd can call it directly, which avoids us
having store a global and document it.
2. Remove the (undocumented, unused, forgotten) --rpc-file="" option to disable
JSON RPC.
3. Move the ickiness of finding the executable path into subd.c, so it doesn't
distract from lightningd.c overview.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This could have been a local static but its used by the run-param test,
so putting it in json.c made things easier.
Signed-off-by: Mark Beckwith <wythe@intrig.com>
Note: Unlike before, this will now accept positional parameters.
Note: In case of error we no longer report the hop number. Is this acceptable?
We still report the name of the bad param and its value.
One option is to log the hop number if param() returns false. This would require
a change to command_fail so it doesn't delete the cmd, so we can still
access cmd->ld->log.
Signed-off-by: Mark Beckwith <wythe@intrig.com>
The `json_tok_X` functions now consistently check the success case first
and call `command_fail` at the end.
Signed-off-by: Mark Beckwith <wythe@intrig.com>
It's probably unnecessary to have this weird way of injecting results
now we have explicit feerate args.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
And, reluctantly, default to bitcoind style.
"It's wrong to be right too soon."
Suggested-by: @cdecker
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Rusty Russell
Christian Decker
arowser
lisa neigut
Wladimir J. van der Laan
Mark Beckwith