Both when we forget about an opening peer, and at startup. We're
going to be relying on this, and the next patch, as we refactor
peer/channel handling to mirror the db.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Combining the two was just awkward, so it's clearer to have separate
functions. And we make the lower-level functions do the escaping.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
The JSON-RPC spec specifies that if the request is unparseable we
should return an error with a NULL id. This is a bit more friendly
than slamming the door in the face.
Signed-off-by: Christian Decker <decker.christian@gmail.com>
As reported by @practicalswift in #945 it is possible to inject
non-printable, or shell escape, characters in a json command, that
will fail to parse and then clear the shell.
Reported-by: @practicalswift
Signed-off-by: Christian Decker <decker.christian@gmail.com>
Now we have wirestring, this is much more natural. And with the
24M length limit, we needn't be so concerned about dumping 64k peer
messages in hex.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
These are now logically arrays of pointers. This is much more natural,
and gets rid of the horrible utxo array converters.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
`activate_peer` does little more than wiring up some txwatches and
asking `gossipd` to reconnect to the peer. If the peer manages to
reconnect before we activate then we would crash.
This just changes the `assert` causing the crash into a conditional
whether we need to reconnect or not.
Signed-off-by: Christian Decker <decker.christian@gmail.com>
Due to the broadcast failure quite a few users are reporting channels
stuck in awaiting lockin. This commit adds a `dev-forget-channel`
command that checks whether the funding outpoint is in the UTXO, and
forgets the channel if not. The UTXO check can be overridden with the
`force` parameter, but that is dangerous.
Signed-off-by: Christian Decker <decker.christian@gmail.com>
We were sideloading it, which is awkward, now it's a field that we can
actually use in the code.
Signed-off-by: Christian Decker <decker.christian@gmail.com>
We currently don't handle LOG_IO properly, and we turn it into a string
before handing it to the ->print function, which makes it ugly for
the case where we're using copy_to_parent_log, and also means in
that case we lose *what peer* the IO is coming from.
Now, we handle the io as a separate arg, which is much neater.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Logging often gets called in error paths, so this is just good hygiene.
Also, log_io does this already.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
libunwind does not accept a NULL parameter for the error callback. It
will simply call into the NULL pointer. So add an error callback.
This makes the crash output somewhat more sensible on FreeBSD, where
there is no libunwind stack trace available:
2018-02-05T20:24:50.598Z lightningd(75556): error getting backtrace: no stack trace because unwind library not available (0)
Signed-off-by: Wladimir J. van der Laan <laanwj@gmail.com>
Maintaining it was always fraught, since the command could go away
if the JSON RPC died. Most recently, it was broken again on shutdown
(see below).
In future we may allow pay commands to block on previous payments, so
it won't even be a 1:1 mapping. Generalize it: keep commands in a
simple list and do a lookup when a payment fails/succeeds.
Valgrind error file: valgrind-errors.5732
==5732== Invalid read of size 8
==5732== at 0x4149FD: remove_cmd_from_hout (pay.c:292)
==5732== by 0x468BAB: notify (tal.c:237)
==5732== by 0x469077: del_tree (tal.c:400)
==5732== by 0x4690C7: del_tree (tal.c:410)
==5732== by 0x46948A: tal_free (tal.c:509)
==5732== by 0x40F1EA: main (lightningd.c:362)
==5732== Address 0x69df148 is 1,512 bytes inside a block of size 1,544 free'd
==5732== at 0x4C2EDEB: free (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==5732== by 0x469150: del_tree (tal.c:421)
==5732== by 0x46948A: tal_free (tal.c:509)
==5732== by 0x4198F2: free_htlcs (peer_control.c:1281)
==5732== by 0x40EBA9: shutdown_subdaemons (lightningd.c:209)
==5732== by 0x40F1DE: main (lightningd.c:360)
==5732== Block was alloc'd at
==5732== at 0x4C2DB8F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==5732== by 0x468C30: allocate (tal.c:250)
==5732== by 0x4691F7: tal_alloc_ (tal.c:448)
==5732== by 0x40A279: new_htlc_out (htlc_end.c:143)
==5732== by 0x41FD64: send_htlc_out (peer_htlcs.c:397)
==5732== by 0x41511C: send_payment (pay.c:388)
==5732== by 0x41589E: json_sendpay (pay.c:513)
==5732== by 0x40D9B1: parse_request (jsonrpc.c:600)
==5732== by 0x40DCAC: read_json (jsonrpc.c:667)
==5732== by 0x45C706: next_plan (io.c:59)
==5732== by 0x45D1DD: do_plan (io.c:387)
==5732== by 0x45D21B: io_ready (io.c:397)
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This is a transitional patch so we can still close channels cleanly;
for want of a better option, I hooked it into --deprecated-apis.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We shouldn't fail negotiation just because they exceeded what we thought
fair: we're better off as long as it's actually <= final commitment fee.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We move it into jsonrpc where it belongs, and make it fail the command.
This means it can tell us exactly what was wrong.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
With the new 'human-readable' mode of lightning-cli, this actually produces
a valid config file. It's a bit hacky though...
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We may need to lookup UTXO entries for other reasons, so here we
disentangle it and make it into its own method.
Signed-off-by: Christian Decker <decker.christian@gmail.com>
Might help alleviate some of the issues of having to run a full-node
on the same machine as `lightningd`.
Signed-off-by: Christian Decker <decker.christian@gmail.com>
Exception: Node /tmp/lightning-t5gxc6gs/test_closing_different_fees/lightning-2/ has memory leaks: [{'value': '0x55caa0a0b8d0', 'label': 'ccan/ccan/tal/str/str.c:90:char[]', 'backtrace': ['ccan/ccan/tal/tal.c:467 (tal_alloc_)', 'ccan/ccan/tal/tal.c:496 (tal_alloc_arr_)', 'ccan/ccan/tal/str/str.c:90 (tal_vfmt)', 'lightningd/log.c:131 (new_log)', 'lightningd/subd.c:632 (new_subd)', 'lightningd/subd.c:686 (new_peer_subd)', 'lightningd/peer_control.c:2487 (peer_accept_channel)', 'lightningd/peer_control.c:674 (peer_sent_nongossip)', 'lightningd/gossip_control.c:55 (peer_nongossip)', 'lightningd/gossip_control.c:142 (gossip_msg)', 'lightningd/subd.c:477 (sd_msg_read)', 'lightningd/subd.c:319 (read_fds)', 'ccan/ccan/io/io.c:59 (next_plan)', 'ccan/ccan/io/io.c:387 (do_plan)', 'ccan/ccan/io/io.c:397 (io_ready)', 'ccan/ccan/io/poll.c:305 (io_loop)', 'lightningd/lightningd.c:347 (main)', '(null):0 ((null))', '(null):0 ((null))', '(null):0 ((null))'], 'parents': ['lightningd/log.c:103:struct log_book', 'lightningd/lightningd.c:43:struct lightningd']}]
Technically, true, but we save more memory by sharing the prefix pointer
than we lose by leaking it.
However, we'd ideally refcount so it's freed if the log is freed and
all the entries using it are pruned from the log book.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This makes much more sense when you ask for a specific peer's log.
Also, we put the peerid rather than pid ().
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We added code to allow a few spurious failures, but it didn't unmark
the request running.
IRC user 'mlz' (@molxyz) provided logs from his stuck-at-old-block lightningd:
lightningd(31981): Adding block 1261159: 00000000da3890ccd0f313a74fccfd4789654b496836da5c28a8d2ad28852264
lightningd(31981): Adding block 1261160: 00000000f70938a33aecbdd7b047cb5cf5b095ea4770c1335acf1859bad1e767
lightningd(31981): bitcoin-cli -testnet estimatesmartfee 2 CONSERVATIVE exited with status 1
Fixes: #749
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
There is an interaction between --ipaddr and --port, namely that the
default port is used when parsing --ipaddr if --port comes after the
--ipaddr, and --port is used if it comes before it. Adding a port to
--ipaddr still trumps everything else, but this way we correctly set
port in the address.
Reported-by: Wladimir J. van der Laan @laanwj
Signed-off-by: Christian Decker <decker.christian@gmail.com>
The JSON connect command wouldn't terminate if peer reconnected
in a state CHANNELD_AWAITING_LOCKIN or above.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Such an htlc is invalid, and will be failed cleanly by our channeld
(which also checks that it meets the minimum amount), but it's
not the master's job to check it, and in fact, it asserts if we were
to try to pay or forward such a thing.
Fixes: #686
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
The peer shouldn't try, and channeld won't try to add it if it does,
but we shouldn't trust it. And it would make our htlc_in_check() code
assert.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Before this patch:
```
$ lightningd/lightningd
lightningd(PID): Creating lightningd dir /root/.lightning (because chdir gave No such file or directory)
lightningd(PID): Creating database
```
After this patch:
```
$ lightningd/lightningd
lightningd(PID): Creating lightningd dir /root/.lightning
lightningd(PID): Creating database
```
delinvoice was orginally documented to only allow deletion of unpaid
invoices, but there might be reasons to delete paid ones or unexpired ones.
But we have to avoid the race where someone pays as it's deleted: the
easiest way is to have the caller tell us the status, and fail if
it's wrong.
Fixes: #477
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Error code is inverted (which makes sense: who returns 'true' on
error?), and anyway there's a leak if we do error.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We're going to have to support multiple channels per peer, even if only
when some are onchain. This would break the current listpeers, so
change it to an array (single element for now).
Other cleanups:
1. Only set connected true if daemon is not onchaind.
2. Only show netaddr if connected; don't make it an array, call it `address`
in comparison with `addresses` in listnodes.
3. Rename `channel` to `short_channel_id`
4. Add `funding_txid` field for voyeurism.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This allows us to add other fields, such as version information,
warnings or invoiceless payments, later.
(Note: the deprecated listinvoice is unchanged)
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This matches the other names, and also the return value is about to change.
This will be removed before release!
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This can be used for upgrades to make sure you're not using deprecated
options, JSON commands, JSON fields, etc.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
For performance, we delay entering the 'wallet_payment' into the db
until we actually commit to the HTLC (when we have to touch the DB
anyway).
This opens a race where we can try to pay twice, and since it's not in
the database yet, we don't notice the duplicate.
So remove the temporary payment field from htlc_out, which was always
an uncomfortable hack, and make the wallet code abstract over the
deferred entry a little by maintaining a 'unstored_payments' list
and incorporating that in results.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We need these to decode any returned errors.
We remove it from struct pay_command too, and load directly from db
when we need it.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We should be saving this, as it's our proof of payment. Also, we return
it if they try to pay again.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This, of course, should never be used. But it helps maintain connections
for the moment while we dig deeper into feerates.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This is a common occurence on pruned nodes. By calling the callback
upon failures, we communicate that we couldn't verify the txoutput. We
fail safe rejecting any channel we can't verify.
Signed-off-by: Christian Decker <decker.christian@gmail.com>
This means we print out the correct path with --debugger, which
can be vital if there are multiple binaries (eg. compiled vs installed).
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
json_get_params does this for us.
Fixes: 78adf0b (pay: allow 'null' msatoshi field.)
Reported-by: ZmnSCPxj
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Pulling up the save call from `peer_save_commitsig_received` into its
caller `peer_got_commitsig` and adding a call to
`peer_sending_commitsig`
Signed-off-by: Christian Decker <decker.christian@gmail.com>
Message buffer `why` is allocated in the `peer` context and also freed when peer is freed.
Only explicitly free the buffer when peer itself is not freed yet.
exit status is not enough to detect spent outputs. gettxout will return a
success exit code and 0 bytes.
Signed-off-by: William Casarin <jb55@jb55.com>
We'll pass this down to gossip and make sure to re-announce/update
channels every so often. This is also used as a pruning timer, i.e.,
channels that have not been updated in 2 x channel-update-interval
will be pruned from the local view.
Signed-off-by: Christian Decker <decker.christian@gmail.com>
Since most callers use positional arguments, we should allow a 'null'
literal where we require no value at all.
Also adds some more value tests.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Paid invoices need to know how much was actually paid: both for the case
where no 'msatoshi' amount was specified, and for the normal case, where
clients are permitted to overpay in order to help them disguise their
payments.
While we migrate the db, we leave this field as 0 for old paid
invoices. This is unhelpful for accounting, but at least clearly
indicates what happened if we find this in the wild.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
'rhash' is the old terminology, but 'payment_preimage' and
'payment_hash' were decided on for the BOLTs, so we should fix that here.
We still use rhash internally, but that's much easier to fix.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Different commands (listinvoice, delinvoice, waitinvoice,
waitanyinvoice) returned different fields, as not all were updated.
This makes them uniform.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This reuses the same code internally, and also now means that we deal
correctly with "any" msatoshi invoices: the old code would a return
'msatoshi' of 0 in that case.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
The manfile and the online help use 'msatoshi', the returned
response uses 'msatoshi', nearly every invoice-related
monetary amount is labelled 'msatoshi' and not 'amount'.
It would be nice if bitcoind had an RPC to do this in one, but that's
a bit much to ask for. We could also hand around proofs, for lite nodes.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
1. htlc->fail has been changed to a u8 *.
2. wallet_get_newindex saves to the db.
3. peer->next_htlc_id is saved to the db in peer_save_commitsig_sent() below.
4. We do store commit in peer_save_commitsig_received(peer, commitnum),
and the fixme below talks about HTLC sigs.
5. We do commit shachain and next_per_commit_point in wallet_shachain_add_hash
and update_per_commit_point respectively.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
All other users of json_get_params(...) check the return value:
```
lightningd/chaintopology.c: if (!json_get_params(buffer, params,
lightningd/chaintopology.c: if (!json_get_params(buffer, params,
lightningd/dev_ping.c: if (!json_get_params(buffer, params,
lightningd/gossip_control.c: if (!json_get_params(buffer, params,
lightningd/invoice.c: if (!json_get_params(buffer, params,
lightningd/invoice.c: if (!json_get_params(buffer, params,
lightningd/invoice.c: if (!json_get_params(buffer, params,
lightningd/invoice.c: if (!json_get_params(buffer, params,
lightningd/invoice.c: if (!json_get_params(buffer, params, "label", &labeltok, NULL)) {
lightningd/invoice.c: if (!json_get_params(buffer, params,
lightningd/jsonrpc.c: if (!json_get_params(buffer, params,
lightningd/pay.c: if (!json_get_params(buffer, params,
lightningd/pay.c: if (!json_get_params(buffer, params,
lightningd/peer_control.c: if (!json_get_params(buffer, params,
lightningd/peer_control.c: if (!json_get_params(buffer, params,
lightningd/peer_control.c: if (!json_get_params(buffer, params,
lightningd/peer_control.c: if (!json_get_params(buffer, params,
lightningd/peer_control.c: if (!json_get_params(buffer, params,
lightningd/peer_control.c: if (!json_get_params(buffer, params,
wallet/walletrpc.c: if (!json_get_params(buffer, params,
wallet/walletrpc.c: if (!json_get_params(buffer, params, "tx", &txtok, NULL)) {
```
