Callers to param() can now optionally set a flag to see if command_fail was
called.
This is necessary because the `cmd` is freed in case of failure.
I spent a bit of time trying to extend the lifetime of the `cmd` to the end
of parse_request(), but the destructors still needed to be called when they
were, and it was getting ugly. So I took this minimal approach.
Signed-off-by: Mark Beckwith <wythe@intrig.com>
Now call param() even for commands that don't accept any parameters.
This is a bugfix of sorts. For example, before you could call:
bitcoin-cli getinfo blah
and the blah parameter would be ignored.
Now you will get an error: "too many parameters: got 1, expected 0"
Signed-off-by: Mark Beckwith <wythe@intrig.com>
Added the concept of a "command mode". The
behavior of param() changes based on the mode.
Added and tested the command mode of CMD_USAGE for
setting the usage of a command without running it.
Only infrastructure and test. No functional changes.
Signed-off-by: Mark Beckwith <wythe@intrig.com>
BOLT 7's been updated to split the flags field in `channel_update`
into two: `channel_flags` and `message_flags`. This changeset does the
minimal necessary to get to building with the new flags.
We couldn't use it before because it asserted dbid was non-zero. Remove
assert and save some code.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Header from folded patch 'fixup!_lightningd__use_hsm_get_client_fd()_helper_for_global_daemons_too.patch':
fixup! lightningd: use hsm_get_client_fd() helper for global daemons too.
Suggested-by: @ZmnSCPxj
That matches the other CSV names (HSM was the first, so it was written
before the pattern emerged).
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
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>
Mark Beckwith
Rene Pickhardt
lisa neigut
Rusty Russell
Christian Decker
arowser
Wladimir J. van der Laan