It's always true for the first hook invocation, but if there is more
than one plugin, it could vanish between the two! In the default configuration, this can't happen.
This bug has been around since v23.02.
Note: we always tell all the plugins about the peer, even if it's
already gone.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Fixed: lightningd: possible crash when peers disconnected if there was more than one plugin servicing the `peer_connected` hook.
Reported-by: https://github.com/santyr
Fixes: https://github.com/ElementsProject/lightning/issues/8858
Avoids guessing what the timeout should be, use a file trigger. This
is more optimal, and should reduce a flake in test_sql under valgrind.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
It uses the hold_invoice plugin to ensure that an HTLC is in flight, but
it tells it to hold the HTLC for "TIMEOUT * 2" which is a big number under CI.
Reduce it to sqrt(TIMEOUT + 1) * 2, which works for local testing (I run
with TIMEOUT=10) and still should be enough for CI (TIMEOUT=180).
Christian reported that the test took 763.00s (!!) under CI.
On my build machine (TIMEOUT=90):
Before:
383.00s
After:
64.38s
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
If l2 hasn't seen l1's node_announcement yet:
```
# Correctly handles missing object.
> assert l2.rpc.sql("SELECT option_will_fund_lease_fee_base_msat,"
" option_will_fund_lease_fee_basis,"
" option_will_fund_funding_weight,"
" option_will_fund_channel_fee_max_base_msat,"
" option_will_fund_channel_fee_max_proportional_thousandths,"
" option_will_fund_compact_lease"
" FROM nodes WHERE HEX(nodeid) = '{}';".format(l1.info['id'].upper())) == {'rows': [[None] * 6]}
E AssertionError: assert {'rows': []} == {'rows': [[None, None, None, None, None, None]]}
E
E Differing items:
E {'rows': []} != {'rows': [[None, None, None, None, None, None]]}
E
E Full diff:
E {
E - 'rows': [
E + 'rows': [],
E ? ++
E - [
E - None,
E - None,
E - None,
E - None,
E - None,
E - None,
E - ],
E - ],
E }
tests/test_plugin.py:4131: AssertionError
```
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
These are explicitly written to check that the values don't accidentally change,
which applies to both old and new styles.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This means:
1. downgrade changes (we no longer fail due to node biases).
2. various deprecations no longer are
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Because l1 and l3 allow localhost as a broadcastable address, they can
try to reconnect. Disable reconnections, so we don't race:
```
> l2.rpc.connect(l3.info['id'], 'localhost', l3.port)
tests/test_plugin.py:4146:
...
elif "error" in resp:
> raise RpcError(method, payload, resp['error'])
E pyln.client.lightning.RpcError: RPC call failed: method: connect, payload: {'id': '035d2b1192dfba134e10e540875d366ebc8bc353d5aa766b80c090b39c3a5d885d', 'host': 'localhost', 'port': 45035}, error: {'code': 402, 'message': 'disconnected during connection'}
```
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
There's one more complaint we can see when plugins get upset:
```
lightningd-1 2026-01-12T06:10:49.317Z **BROKEN** plugin-cln-xpay: askrene-create-layer failed with {"code":-4, "message":"Plugin terminated before replying to RPC call."}
```
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
It's not reliable:
```
# We should have deferred hook update at least once!
> l2.daemon.wait_for_log("UNUSUAL plugin-dep_b.py: Deferring registration of hook htlc_accepted until it's not in use.")
tests/test_plugin.py:2646:
...
if self.is_in_log(r):
print("({} was previously in logs!)".format(r))
> raise TimeoutError('Unable to find "{}" in logs.'.format(exs))
E TimeoutError: Unable to find "[re.compile("UNUSUAL plugin-dep_b.py: Deferring registration of hook htlc_accepted until it's not in use.")]" in logs.
```
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We restart the nodeL if the coin_movements.py plugin hasn't processed the
notification yet, it will be incorrect:
```
> assert account_balance(l2, chanid_1) == 100001001
E AssertionError: assert 150_001_001msat == 100_001_001
E + where 150001001msat = account_balance(<fixtures.LightningNode object at 0x7f0634e1eb00>, '39ac52c818c5304cf0664940ff236c4e3f8f4ceb8993cb1491347142d61b62bc')
```
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We used to just run these without valgrind, but we already run them in
CI (which sets SLOW_MACHINE) without valgrind, so this just doubles
up.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
When installed, the name is `lightning-hsmtool`. We actually copy
`tools/hsmtool` to `tools/lightning-hsmtool` but that's a silly step
which we should get rid of.
So:
1. Make sure our documentation always refers to it as lightning-hsmtool.
2. Make sure our tests invoke it as `lightning-hsmtool`.
3. Rename the C file.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We don't expect an internal command to take 5 seconds to service
without explicitly pausing: if it does, log at a higher level.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
If we add a new hook, not at the end, while hooks are getting called,
then iteration could be messed up (e.g. calling a plugin twice, or
skipping one).
The simplest thing is to defer updates until nobody is calling the
hook. In theory this could livelock, in practice it won't.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
bc4bb2b0ef "libplugin: use jsonrpc_io logic for sync requests too."
changed this message, and test was not updated.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Added: JSON-RPC: `listpeerchannels` `funding` object `withheld` flag, and `listclosedchannels` `funding_withheld` flags, indicating fundchannel_complete was called with the `withheld` parameter true.
Changelog-Added: JSON-RPC: `psbt` field in `funding` in listpeerchannels, and `funding_psbt` in listclosedchannels.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
For example, `autoclean-once failedforwards` would count every non-failed forwards
as "uncleaned".
This is both technically correct and completely useless.
Changelog-Fixed: JSON-RPC: `autoclean-once` returns "uncleaned" number reflecting number of candidates which were too new to be cleaned, not all records we didn't delete.
Fixes: https://github.com/ElementsProject/lightning/issues/8632
Reported-by: @grubles and several other sharp-eyed users.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We also document this in the listnetworkevents command itself.
The test_autoclean_once was getting repetitive, so I cleaned that
up too.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Added: `autoclean` will remove networkevents after 30 days by default.
```
lightningd-1 2025-10-27T11:26:04.285Z **BROKEN** plugin-bcli: bitcoin-cli exec failed: Argument list too long
```
Use -stdin to bitcoin-cli: we can then handle arguments of arbitrary length.
Fixes: https://github.com/ElementsProject/lightning/issues/8634
Changelog-Fixed: plugins: `bcli` would fail with "Argument list too long" when sending a giant tx.
Update the exposesecret plugin to work with the new unified HSM secret
format that supports BIP39 mnemonics.
Changelog-Added - exposesecret now has a mnemonic field
Changelog-Added: `hsmtool` now supports hsm_secret files using a 12-word mnemonic.
Changelog-Removed: hsmtool support for mnemonics in non-english languages removed.
This significantly speeds up the query which bookkeeper often does:
"SELECT created_index"
" FROM channelmoves"
" WHERE payment_hash = X'%s'"
" AND credit_msat = %"PRIu64
" AND created_index <= %"PRIu64,
On large databases this scan is expensive, and a payment_hash index
cuts it down a great deal. It does take longer to load the channelmoves
in the first place though (about 3x).
Before:
$ while sleep 10; do wc -l /tmp/bkpr-progress; done
169505 /tmp/bkpr-progress
196010 /tmp/bkpr-progress
219370 /tmp/bkpr-progress
235671 /tmp/bkpr-progress
244242 /tmp/bkpr-progress
255362 /tmp/bkpr-progress
265636 /tmp/bkpr-progress
276966 /tmp/bkpr-progress
284451 /tmp/bkpr-progress
288836 /tmp/bkpr-progress
296578 /tmp/bkpr-progress
304571 /tmp/bkpr-progress
After:
$ while sleep 10; do wc -l /tmp/bkpr-progress; done
161421 /tmp/bkpr-progress
238273 /tmp/bkpr-progress
281185 /tmp/bkpr-progress
305787 /tmp/bkpr-progress
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Changed: plugins: the sql plugin now keeps an index on `channelmoves` by `payment_hash`.
1. Establish a channel with l3; we already have one with l2.
2. Don't bother generating 6 more blocks (fundchannel ensures it's mined).
3. Allow htlcs to be empty: Whitslack reported that happens for him
4. Use only_one() to access where we insist there is only one element in the list.
5. Tighten tests to assert the exact contents, not just test some.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Fixes: https://github.com/ElementsProject/lightning/issues/8497
After much thought and mis-steps, I chose a simple solution: open another fd
for sync comms. It's almost impossible to know what state the async one is in.
jsonrpc_request_sync() is enhanced to return a valid tal object, as the current
behaviour of returning a pointer to inside an array was surprising.
Changelog-Changed: libplugin: you can now call the synchronous API functions at any time (not just in the init callback).
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Simply wait if there's one going already. This is a minor
optimization, but critical for the case where we do partial refreshes
asynchonously (rather than deleting everything and reloading). This
is currently only coinmoves and chainmoves, but the duplicated effort
is a waste everywhere.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
If we do this, we get a database error (now we try to refresh
intelligently, is this is currently only chainmoves / channelmoves).
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
It's a unique integer, and very useful for querying changes. Unlike
our generated rowid, it's *stable* across queries.
We still need an explicit rowid column for list commands which don't
(currently) have this.
Here's the documentation diff:
@@ -85,69 +85,69 @@
TABLES
------
-Note that the first column of every table is a unique integer called `rowid`: this is used for related tables to refer to specific rows in their parent. sqlite3 usually has this as an implicit column, but we make it explicit as the implicit version is not allowed to be used as a foreign key.
+Note that tables which have a `created_index` field use that as the primary key (and `rowid` is an alias to this), otherwise an explicit `rowid` integer primary key is generated, whose value changes on each refresh. This field is used for related tables to refer to specific rows in their parent. (sqlite3 usually has this as an implicit column, but we make it explicit as the implicit version is not allowed to be used as a foreign key).
The following tables are currently supported:
- `bkpr_accountevents` (see lightning-bkpr-listaccountevents(7))
@@ -119,14 +119,14 @@
- `payment_id` (type `hex`, sqltype `BLOB`)
- `chainmoves` indexed by `account_id` (see lightning-listchainmoves(7))
- - `created_index` (type `u64`, sqltype `INTEGER`)
+ - `created_index` (type `u64`, sqltype `INTEGER PRIMARY KEY`)
- `account_id` (type `string`, sqltype `TEXT`)
- `credit_msat` (type `msat`, sqltype `INTEGER`)
- `debit_msat` (type `msat`, sqltype `INTEGER`)
- `timestamp` (type `u64`, sqltype `INTEGER`)
- `primary_tag` (type `string`, sqltype `TEXT`)
- related table `chainmoves_extra_tags`
- - `row` (reference to `chainmoves.rowid`, sqltype `INTEGER`)
+ - `row` (reference to `chainmoves.created_index`, sqltype `INTEGER`)
- `arrindex` (index within array, sqltype `INTEGER`)
- `extra_tags` (type `string`, sqltype `TEXT`)
- `peer_id` (type `pubkey`, sqltype `BLOB`)
@@ -139,7 +139,7 @@
- `blockheight` (type `u32`, sqltype `INTEGER`)
- `channelmoves` indexed by `account_id` (see lightning-listchannelmoves(7))
- - `created_index` (type `u64`, sqltype `INTEGER`)
+ - `created_index` (type `u64`, sqltype `INTEGER PRIMARY KEY`)
- `account_id` (type `string`, sqltype `TEXT`)
- `credit_msat` (type `msat`, sqltype `INTEGER`)
- `debit_msat` (type `msat`, sqltype `INTEGER`)
@@ -204,7 +204,7 @@
- `last_stable_connection` (type `u64`, sqltype `INTEGER`)
- `forwards` indexed by `in_channel` and `in_htlc_id` (see lightning-listforwards(7))
- - `created_index` (type `u64`, sqltype `INTEGER`)
+ - `created_index` (type `u64`, sqltype `INTEGER PRIMARY KEY`)
- `in_channel` (type `short_channel_id`, sqltype `TEXT`)
- `in_htlc_id` (type `u64`, sqltype `INTEGER`)
- `in_msat` (type `msat`, sqltype `INTEGER`)
@@ -222,7 +222,7 @@
- `htlcs` indexed by `short_channel_id` and `id` (see lightning-listhtlcs(7))
- `short_channel_id` (type `short_channel_id`, sqltype `TEXT`)
- - `created_index` (type `u64`, sqltype `INTEGER`)
+ - `created_index` (type `u64`, sqltype `INTEGER PRIMARY KEY`)
- `updated_index` (type `u64`, sqltype `INTEGER`)
- `id` (type `u64`, sqltype `INTEGER`)
- `expiry` (type `u32`, sqltype `INTEGER`)
@@ -242,7 +242,7 @@
- `bolt12` (type `string`, sqltype `TEXT`)
- `local_offer_id` (type `hash`, sqltype `BLOB`)
- `invreq_payer_note` (type `string`, sqltype `TEXT`)
- - `created_index` (type `u64`, sqltype `INTEGER`)
+ - `created_index` (type `u64`, sqltype `INTEGER PRIMARY KEY`)
- `updated_index` (type `u64`, sqltype `INTEGER`)
- `pay_index` (type `u64`, sqltype `INTEGER`)
- `amount_received_msat` (type `msat`, sqltype `INTEGER`)
@@ -408,7 +408,7 @@
- `features` (type `hex`, sqltype `BLOB`)
- `sendpays` indexed by `payment_hash` (see lightning-listsendpays(7))
- - `created_index` (type `u64`, sqltype `INTEGER`)
+ - `created_index` (type `u64`, sqltype `INTEGER PRIMARY KEY`)
- `id` (type `u64`, sqltype `INTEGER`)
- `groupid` (type `u64`, sqltype `INTEGER`)
- `partid` (type `u64`, sqltype `INTEGER`)
Changelog-Changed: Plugins: `sql` tables `forwards`, `htlcs`, `invoices`, `sendpays` all use `created_index` as their primary key (and `rowid` is now an alias to this).
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
And note the other commands in See Also section.
Note that this means handling the "outpoint" type.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Added: JSON-RPC: `sql` plugin now supports `chainmoves` and `channelmoves` tables.
All the core notifications changed over to wrapping the notification
fields in an object with the name of the notification, but notifications
from plugins were missed.
Changelog-Added: Plugins: `channel_hint_update`, `pay_failure` and `pay_success` notifications now have objects of the same name containing the expected fields.
Changelog-Deprecated: Plugins: `channel_hint_update`, `pay_failure` and `pay_success` notification fields outside the same-named object.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Rather than forcing them to wrap their parameters in a "payload"
sub-object, copy in params directly. We include the "origin" field
one level up, if they care.
The next patch restores compatibility for the one place we currently use
them, which is the pay plugin.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Deprecated: pyln-client: plugin custom notifications origins and payload (use parameters directly)
For older lightningd, we copy field into the raw dict, for newer we recreate the old
"payload" member.
We do fix up the custom_notification test which set params to a string instead of a dict:
that's just weird!
We also change the hacky parsing to proper dict extraction.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Changed: pyln-client: plugin notifications parameters now exposed directly, not wrapped in `params` object.
We always prefer to omit fields rather than use 'null' (or unknown!).
Note that before this, the schema was broken, so we have to put a special
exemption in for that case.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
