`struct lightningd` is not completely initialized, so we added a
"migration_context" which only had some of the fields. But we ended
up handing in `struct lightningd` anyway, so I don't think this
complexity is worthwhile.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
At the moment only lightingd needs it, and this avoids missing any
places where we do bip32 derivation.
This uses a hsm capability to mean we're backwards compatible with older
hsmds.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Added: Protocol: we now always double-check bitcoin addresses are correct (no memory errors!) before issuing them.
It's needed as the db and wallet is being set up (db migrations), so
it's simpler this way to always use ld->bip32_base for the next patch.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Importantly, adds the version number at the *front* to help future
parsing.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Header from folded patch 'fix-hsm-check-pubkey.patch':
fixup! hsmd: capability addition: ability to check pubkeys.
e778ebb9af ("wallet: only log broken if we
have duplicate scids in channels.") downgraded the fatal() to a broken
log message, but the user reports it still won't start up.
Perhaps they're hitting the fatal() outside the loop? (And we're
not getting that output).
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Missed a DEFAULT in the db clause.
Feb 15 16:02:12 citrine lightningd[902093]: Accessing a null column lease_satoshi/15 in query SELECT funding_tx_id, funding_tx_outnum, funding_feerate, funding_satoshi, our_funding_satoshi, funding_psbt, last_tx, last_sig, funding_tx_remote_sigs_received, lease_expiry, lease_commit_sig, lease_chan_max_msat, lease_chan_max_ppt, lease_blockheight_start, lease_fee, lease_satoshi FROM channel_funding_inflights WHERE channel_id = ? ORDER BY funding_feerate
Fixes#6016
People are upgrading to 22.11.1 not, and in some configurations like the one
mentioned in the issue, we should
put some info information in the log when we are not able to upgrade.
Signed-off-by: Vincenzo Palazzo <vincenzopalazzodev@gmail.com>
It's not likely but possible that the node's settings will shift btw a
start and an RBF; we persist the setting to the database so we don't
lose it.
Right now holding onto it forever is kind of extra but maybe we'll
reuse the setting for splices? idk.
Should this be a channel type??
technically we don't need this info after the channel opens, but for any
subsequent RBF (and maybe splice?) we need to remember what the
open/accept peer signaled
We need to be able to only use non-wrapped inputs for v2/interactive tx
protocol.
Changelog-Added: JSONRPC: `fundpsbt` option `nonwrapped` filters out p2sh wrapped inputs
We didn't actually populate them properly, and the real annotations
are on inputs and outputs.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-EXPERIMENTAL: JSON-RPC: `listtransactions` `channel` and `type` field removed at top level.
We only ever use this table for output and input transactions: indeed, my node
doesn't have any annotation types 0.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
After connecting 100,000 peers with one channel each (not all at
once!), we see various places where we exhibit O(N^2) behaviour.
Fix these by keeping a map of id->peer instead of a simple
linked-list.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Rework the logic of the version check used in the
database migration, and make sure
that it is full functional to avoid confusion
at release time.
Changelog-Fixed: database: Correctly identity official release versions for database upgrade.
Reported-by: @urza
Signed-off-by: Vincenzo Palazzo <vincenzopalazzodev@gmail.com>
We're soon going to call json_add_unsaved_channel and
json_add_uncommitted_channel from a new place, where we want the peer
state directly included.
Based on patch by @vincenzopalazzo.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
I discovered this while reworking the CI workflow, and it seems like
the HTLC queries do not order, while the tests assume a specific
order. This matches sqlite3 which without an explicit ORDER clause
will use insertion order, while postgres does not keep things in
insertion order, thus breaking the assumption. Ordering by `id`
re-establishes that implicit assumption
Changelog-Changed: postgres: Ordering of HTLCs in `listhtlcs` are now ordered by time of creation
Changelog-Removed: JSON-RPC: `fundpsbt`/`utxopsbt` `reserve` must be a number, not bool (deprecated v0.11.0)
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This was reported, but the channel was closed. So however we ended
up with a duplicate, we're no *worse* off than we were before migration?
Fixes: #5760
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We have a primary key that is spanning the `in_channel_id` and the
`in_htcl_id`. The latter gets set to NULL when the HTLC and channel
gets deleted, so we coalesce with a random large number that is
unlikely to collide for the primary key.
Ubuntu clang 15.0.2-1 complains:
```
wallet/wallet.c:280:6: error: variable 'i' set but not used
[-Werror,-Wunused-but-set-variable]
int i;
^
wallet/wallet.c:339:6: error: variable 'i' set but not used
[-Werror,-Wunused-but-set-variable]
int i;
^
wallet/wallet.c:4768:9: error: variable 'count' set but not used
[-Werror,-Wunused-but-set-variable]
size_t count;
^
```
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We no longer use offers for "I want to send you money", but we'll use
invoice_requests directly. Create a new table for them, and
associated functions.
The "localofferid" for "pay" and "sendpay" is now "localinvreqid".
This is an experimental-only option, so document the change under
experimental only.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-EXPERIMENTAL: JSON-RPC: `pay` and `sendpay` `localofferid` is now `localinvreqid`.
This current spec is not strict enough: we might complain that the
next peer is not connected, for example, which leaks information.
So return WIRE_INVALID_ONION_BLINDING even if we're the first hop
on the path, to be safe.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Changed: JSON-RPC: `listfunds` now lists coinbase outputs as 'immature' until they're spendable
Changelog-Changed: JSON-RPC: UTXOs aren't spendable while immature
We still have an "enum forward_style" for the database, where old-style
forwards can still exist.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Removed: Protocol: we no longer forward HTLCs with legacy onions.
We have to allow them (as otherwise `fees_collected_msat` in getinfo breaks),
but it means that actually, in_htlc_id might be missing in listforwards
(also, out_htlc_id might be missing, which we didn't catch before).
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Fixes: #5628
Otherwise what the hook sees is actually a lie, and if it sets it
we might override it.
The side effect is that we add an explicit "forward_to" field, and
allow hooks to override it. This lets a *hook* control channel
choice explicitly.
Changelod-Added: Plugins: `htlc_accepted_hook` return can specify what channel to forward htlc to.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This is actually what the autoclean plugin wants, especially since
you can't otherwise delete a payment which has failed then succeeded.
But insist on neither or both being specified, at least for now.
Changelog-Added: JSON-RPC: `delpay` takes optional `groupid` and `partid` parameters to specify exactly what payment to delete.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Using `listfowards` for this wrong; expose this directly if people
care (and unlike listforwards, which could be deleted, we have to
remember these while the channel is still open!).
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Added: JSON-RPC: `listhtlcs` new command to list all known HTLCS.
And document that we never know payment_hash.
Changelog-Added: JSON-RPC: `listforwards` now shows `in_htlc_id` and `out_htlc_id`
Changelog-Changed: JSON-RPC: `listforwards` now never shows `payment_hash`; use `listhtlcs`.
Normally, we'd use the delete_columns function to remove the old
`short_channel_id` string field, *but* we can't do that for sqlite, as
there are other tables with references to it. So add a FIXME to do
it once everyone has upgraded to an sqlite3 which has native support
for column deletion.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Although it's deprecated already (because it stores as string), it's
better to make the name explicit. And create a new helper which stores as BIGINT.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This one directly contains the scids of the channels involved, not
references, so can outlive the channels. As a side-effect, however,
it now never lists `payment_hash`. Having it listed (via join) is not
possible as it is a *string* in the channels table, and difficult
anyway because of channel aliases.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
contrib/giantnode.py shows we're spending a lot of time looking up
payments by payment_hash: sendpays does it to see if we've already
paid, and bookkeeper does it in listsendpays:
```
- 94.52% 0.00% lightningd lightningd [.] read_json
- 94.52% read_json
- 94.48% parse_request
- 94.46% plugin_hook_call_rpc_command
- plugin_hook_call_
- rpc_command_hook_final
- 94.46% command_exec
- 49.08% json_sendpay
- 49.01% send_payment
- 48.86% send_payment_core
- 48.84% wallet_payment_list
- 48.80% db_step
+ db_sqlite3_step
- 45.38% json_listsendpays
- 45.36% wallet_payment_list
- 45.30% db_step
+ 45.30% db_sqlite3_step
```
This doesn't actually make much of a difference, so see next patch.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This allows GDB to print values, but also allows us to use them in
'case' statements. This wasn't allowed before because they're not
constant terms.
This also made it clear there's a clash between two error codes,
so move one.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Changed: JSON-RPC: Error code from bcli plugin changed from 400 to 500.
First, merge the _ahf_ and non-ahf interfaces.
Second, remove the always-NULL txs->cmd field.
Then, add optional id_prefix for bitcoind_sendrawx, so if it's
triggered by a command (e.g. "withdraw") it's shown correctly in logs.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This is cleaner because, the `remote_addr` and `discovered_ip` are
related but two different things.
Within connectd and lightningd we use the peers `remote_addr` feature
to validate (and guess a port) to be used for IP discovery.
Also when a peer reports us a `remote_addr`, this is given to the plugin API
via the `peer_connected` hook. The network port here is not modified for
godd reason! This can be used i.e. to detect if we are behind a NAT.
But once lightningd figures enough peers report the same `remote_addr`,
it sets the port to the selected network and tells gossipd to use that for
`node_announcement` updates.
Hence, within gossipd, there is no (should not be) `remote_addr`.
Changelog-None
This is a good sanity check that users understand that if they upgrade
to master mid-cycle they can't go back!
Suggested-by: @wtogami
Changelog-Added: Config: `--database-upgrade=true` required if a non-release version wants to (irrevocably!) upgrade the db.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
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 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.
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>
1. If the tool changes, you need to regenerate since the output may
change.
2. This didn't just filter that out, ignored all but the first
dependency, which made bisecting the bookkeeper plugin a nightmare:
it didn't regenerate the .po file, causing random crashes.
If we want this, try $(filter-out tools/fromschema.py) instead. But I
don't think we want that.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We annotate them with UNNEEDED, which is legal but weird, but it
makes gcc (at least 11.2.0) complain about shadowing:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106424
I simply removed the names.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
If you build a PSBT externally from CLN and attempt to sign for the
output, we would crash. Now we don't crash.
Changelog-Changed: JSON-RPC: `signpsbt` will now add redeemscript + witness-utxo to the PSBT for an input that we can sign for, before signing it.
Fixes#5499 ?
Postgresql actually checks, and fails.
It's unclear why this field is an INTEGER (and u32) when it's a BIGINT
in db here (it's an INTEGER in the channels table, just a BIGINT in the
channel_funding_inflights table).
Changelog-Fixed: db: postgresql crash on startup when dual-funding lease open is pending with "s32 field doesn't match size: expected 4, actual 8"
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We used to tell connectd to remember our connect delay, and hand it
back (increased if necessary).
Instead, simply record when we last tried to connect. If it was less
than 10 minutes ago, double delay (up to 5 minutes max), otherwise
reset delay to 1 second.
This covers all scenarios: whether we reconnect then immediately
disconnect, or never successfully connect, it doesn't matter.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Fixes: #5453
If you get the wrong hsm_secret, your node_id will change, and
peers won't know who you are, bitcoind will reject your transaction
signatures, and other madness.
Catch this as soon as it happens, by storing our node_id in the db.
Suggested-by: @cdecker, @fiatjaf
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Changed: Config: `lightningd` will refuse to start with the wrong node_id (i.e. hsm_secret changes).
Connectd already does this when we *receive* an error or warning, but
now do it on send. This causes some slight behavior change: we don't
disconnect when we close a channel, for example (our behaviour here
has been inconsistent across versions, depending on the code).
When connectd is told to disconnect, it now does so immediately, and
doesn't wait for subds to drain etc. That simplifies the manual
disconnect case, which now cleans up as it would from any other
disconnection when connectd says it's disconnected.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This allows us to detect when lightningd hasn't seen our latest
disconnect/reconnect; in particular, we would hit the following pattern:
1. lightningd says to connect a subd.
2. connectd disconnects and reconnects.
3. connectd reads message, connects subd.
4. lightningd reads disconnect and reconnect, sends msg to connect to subd again.
5. connectd asserts because subd is alreacy connected.
This way connectd can tell if lightningd is talking about the previous
connection, and ignoere it.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Before this patch:
1. connectd says it's connected (peer_connected)
2. we tell connectd we want to talk about each channel (peer_make_active)
3. connectd gives us an fd for each channel, and we connect it to a subd (peer_active)
4. OR, connectd says it sent something about a channel we didn't tell it about, with an fd (peer_active)
Now:
1. connectd says it's connected (peer_connected)
2. we start all appropriate subds and tell connectd to what channels/fds (peer_connect_subd).
3. if connectd says it sent something about a channel we didn't tell it about, we either tell
it to hang up (peer_final_msg), or connect a new opening daemon (peer_connect_subd).
This is the minimal-size patch, which is why we create socket pairs in
so many places to use the existing functions. Many cleanups are
possible, since the new flow is so simple.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
First, connectd tells us the peer has connected, and we call the connected hook,
and if it says it's fine, we are actually connected and we fire off notifications.
Of course, we could be disconnected while in the connected hook, and that would
mean we tell people about a connection which is no longer current.
Make this clear with a tristate: if we're not marked disconnected by
the time the hooks finish, we're good. It also gives us a cleaner
"connect" command return when we connected but disconnected before
processing.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
It's directly a product of "does it have a current owner subdaemon"
and "does that subdaemon talk to peers", so create a helper function
which just evaluates that instead.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
There are hardly any lightningd-specific JSON functions: all that's left
are the feerate ones, and there's already a comment that we should have
a lightningd/feerate.h.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We have them split over common/param.c, common/json.c,
common/json_helpers.c, common/json_tok.c and common/json_stream.c.
Change that to:
* common/json_parse (all the json_to_xxx routines)
* common/json_parse_simple (simplest the json parsing routines, for cli too)
* common/json_stream (all the json_add_xxx routines)
* common/json_param (all the param and param_xxx routines)
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
They're not logically connected: we can know where they wanted to
go, but we didn't send it.
Where possible, it's the scid *they asked for*; otherwise, it's the
scid or fallback to the alias, but do this in the *caller*, not by
overriding inside wallet_forwarded_payment_add.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Not only can the outgoing edge be a zeroconf channel, it can also be
the incoming channel. So we revert to the usual trick of using the
local alias if the short_channel_id isn't known yet.
We use the LOCAL alias instead of the REMOTE alias even though the
sender likely used the REMOTE alias to refer to the channel. This is
because we control the LOCAL alias, and we keep it stable during the
lifetime of the channel, whereas the REMOTE one could change or not be
there yet.
`alias_local` is generated locally and sent to the peer so it knows
what we're calling the channel, while `alias_remote` is received by
the peer so we know what to include in routehints when generating
invoices.
We had json_add_amount_msat_only(), which was designed to be used to
print out msat fields, if we had sats.
However, we misused it, so split it into the three different cases:
1. json_add_amount_sat_msat: We are using it correctly, with a field called
xxx_msat.
2. json_add_amount_sats_deprecated: We were using it wrong, so deprecate
the old field and create a new one which does end in _msat.
3. json_add_sats: we were using it to hand sats as a JSON parameter to an
interface, where "XXXsat".
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Deprecated: Plugins: `rbf_channel` and `openchannel2` hooks `their_funding` (use `their_funding_msat`)
Changelog-Deprecated: Plugins: `openchannel2` hook `dust_limit_satoshis` (use `dust_limit_msat`)
Changelog-Deprecated: Plugins: `openchannel` hook `funding_satoshis` (use `funding_msat`)
Changelog-Deprecated: Plugins: `openchannel` hook `dust_limit_satoshis` (use `dust_limit_msat`)
Changelog-Deprecated: Plugins: `openchannel` hook `channel_reserve_satoshis` (use `channel_reserve_msat`)
Changelog-Deprecated: Plugins: `channel_opened` notification `amount` (use `funding_msat`)
Changelog-Deprecated: JSON-RPC: `listtransactions` `msat` (use `amount_msat`)
Changelog-Deprecated: Plugins: `htlc_accepted` `forward_amount` (use `forward_msat`)
Rusty Russell
Kristaps Kaupe
niftynei
Vincenzo Palazzo
Christian Decker
Michael Schmoock
adi2011