Apparently we had two private channel announcements (the !private assert
failed). While this shouldn't happen, don't crash because of it.
Fixes: #5578
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Fixed: Plugins: topology plugin could crash when it sees duplicate private channel announcements.
We can (and probably should!) allocate this off tmpctx rather than
off the response.
Also, json_add_invoice doesn't actually match the normal pattern,
so fix that.
```
E ValueError:
E Node errors:
E Global errors:
E - Node /tmp/ltests-_vbj8az8/test_pay_fail_unconfirmed_channel_1/lightning-2/ has memory leaks: [
E {
E "backtrace": [
E "ccan/ccan/tal/tal.c:442 (tal_alloc_)",
E "wallet/invoices.c:81 (wallet_stmt2invoice_details)",
E "wallet/invoices.c:697 (invoices_get_details)",
E "wallet/wallet.c:2946 (wallet_invoice_details)",
E "lightningd/invoice.c:1296 (json_add_invoices)",
E "lightningd/invoice.c:1370 (json_listinvoices)",
E "lightningd/jsonrpc.c:625 (command_exec)",
E "lightningd/jsonrpc.c:753 (rpc_command_hook_final)",
E "lightningd/plugin_hook.c:280 (plugin_hook_call_)",
E "lightningd/jsonrpc.c:841 (plugin_hook_call_rpc_command)",
E "lightningd/jsonrpc.c:938 (parse_request)",
E "lightningd/jsonrpc.c:1035 (read_json)",
E "ccan/ccan/io/io.c:59 (next_plan)",
E "ccan/ccan/io/io.c:407 (do_plan)",
E "ccan/ccan/io/io.c:417 (io_ready)",
E "ccan/ccan/io/poll.c:453 (io_loop)",
E "lightningd/io_loop_with_timers.c:22 (io_loop_with_timers)",
E "lightningd/lightningd.c:1194 (main)",
E "../csu/libc-start.c:308 (__libc_start_main)"
E ],
E "label": "wallet/invoices.c:81:struct invoice_details",
E "parents": [
E "common/json_stream.c:41:struct json_stream",
E "ccan/ccan/io/io.c:91:struct io_conn **NOTLEAK**",
E "lightningd/lightningd.c:107:struct lightningd"
E ],
E "value": "0x55a57a77de08"
E }
E ]
```
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This avoids having to escape | or &, though we still allow that for
the deprecation period.
To detect deprecated usage, we insist that alternatives are *always*
an array (which could be loosened later), but that also means that
restrictions must *always* be an array for now.
Before:
```
# invoice, description either A or B
lightning-cli commando-rune '["method=invoice","pnamedescription=A|pnamedescription=B"]'
# invoice, description literally 'A|B'
lightning-cli commando-rune '["method=invoice","pnamedescription=A\\|B"]'
```
After:
```
# invoice, description either A or B
lightning-cli commando-rune '[["method=invoice"],["pnamedescription=A", "pnamedescription=B"]]'
# invoice, description literally 'A|B'
lightning-cli commando-rune '[["method=invoice"],["pnamedescription=A|B"]]'
```
Changelog-Deprecated: JSON-RPC: `commando-rune` restrictions is always an array, each element an array of alternatives. Replaces a string with `|`-separators, so no escaping necessary except for `\\`.
JSON needs to escape \, since it can't be in front of anything unexpected,
so no \|. So we need to return \\ to \, and in theory handle \n etc.
Changelog-Fixed: JSON-RPC: `commando-rune` now handles \\ escapes properly.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
1. It depends on both request and reply schemas.
2. Wildcards aren't natively expanded in make, so use $(wildcard).
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
The gossip_store version byte was unaccounted for in the initial traversal
of gossip_store_end. This lead to an offset and a bogus message length
field. As a result, an early portion of the gossip_store could have been
skipped, potentially leading to gossip propagation issues downstream.
Fixes#5572#5565
Changelog-fixed: proper gossip_store operation may resolve some previous gossip propagation issues
The completion of a successful payment is defined as the first
completion of a successful part, hence we just collect the minimum
completion time of successes.
Changelog-Added: JSON-RPC: `pay` and `listpays` now lists the completion time.
test_bookkeeping_closing_trimmed_htlcs fails to find 'all outputs
resolved' occassionally, seems like it's because the
OUR_DELAYED_TO_WALLET doesn't make it into the mempool before we start
mining blocks?
So here make sure there's something in the mempool before before we
start making new blocks.
We were double counting channel lease fees because we were double firing
the channel open event sequence (so to speak). If we don't report
balances for unopened channels, we don't have this problem?
Changelog-Changed: Plugins: `balance_snapshot` notification does not send balances for channels that aren't locked-in/opened yet
Reproduce crash for #5557!
If we record the channel open because bookkeeper was added after the
channel open request started but the channel confirms later, we end up
with re-recording any associated push or leased fees (paid or rcvd).
In the case where you've paid for these fees, your channel balance goes
negative and the node crashes the next time you call `listbalances`.
Reported-by: @chrisguida
Be more graceful in shutting down: this should fix the issue where
bookkeeper gets upset that its commands are rejected during shutdown,
and generally make things more graceful.
1. Stop any new RPC connections.
2. Stop any per-peer daemons (channeld, etc).
3. Shut down plugins.
4. Stop all existing RPC connections.
5. Stop global daemons.
6. Free up peer, chanen HTLC datastructures.
7. Close database.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Changed: Plugins: RPC operations are now still available during shutdown.
Because it used internal routines, it didn't pass operations through the
db hook! So make it use the generic routines, with the twist that they
are not translated.
And when we use this in a migration hook, we're actually in a
transaction.
This, in turn, introduces an issue: we need to be outside a transaction
to "PRAGMA foreign_keys = OFF", but completing the transaction when
there is a db hook actually enters the io loop, freeing the tmpctx!
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This broke with COPTFLAGS="-flto -O3", and so I took a look (it
complains more than normal because main isn't there). We should never
be running update-mocks except on programs expected to compile: in
this case, that's tools/test/run-test-wire.c.
Remove the code which tries to run this, which also means
non-developers won't be running update-mocks!
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This includes the recommendation that we use 10 minute grace period,
so add quotes to where we use that.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This alters the billboard, but that's a human-readable thing so not
noted in CHANGELOG.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Changed: JSON-RPC: `listpeers` `status` now refers to "channel ready" rather than "funding locked" (BOLT language change for zeroconf channels)
Changelog-Added: JSON-RPC: `channel_opened` notification `channel_ready` flag.
Changelog-Deprecated: JSON-RPC: `channel_opened` notification `funding_locked` flag (use `channel_ready`: BOLTs namechange).
This contains the zeroconf stuff, with funding_locked renamed to
channel_ready. I change that everywhere, and try to fix up the
comments.
Also the `alias` field is called `short_channel_id`.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Changed: Protocol: `funding_locked` is now called `channel_ready` as per latest BOLTs.
This would be more effective if we didn't *merge* in the specs repo,
but still.
Usage: ./devtools/bolt-catchup.sh
It goes through one commit at a time, up to current HEAD, and stops when there
are changes, or quotes change.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Removed: `hsmtool`: hsm_secret (ignored) on cmdline for dumponchaindescriptors (deprecated in v0.9.3)
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
It was deprecated in v0.10.1, but only one channel on the network
doesn't set it now anyway, and we'll be ignoring that soon.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We can do this now the function is cleaned up.
Always better to do the work inside param() since then `check`
gets the benefit.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Removed: Old order of the `status` parameter in the `listforwards` rpc command (deprecated in v0.10.2)
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Removed: JSONRPC: RPC framework now requires the `"jsonrpc"` property inside the request (deprecated in v0.10.2)
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Somehow we missed this deprecation, found by grep.
Changelog-Removed: JSON API: Removed double wrapping of `rpc_command` payload in `rpc_command` JSON field (deprecated v0.8.2)
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>