Commit Graph

77 Commits

Author SHA1 Message Date
Rusty Russell
66741dda79 plugins/bkpr/test/run-recorder: don't hand NULL cmd.
ubsan complains that we declared a function not to take NULL.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2025-08-19 13:37:50 +09:30
Rusty Russell
f8a44d911d bkpr: restore run-recorder.
This requires us to turn "sql" calls into calls to a local db, which
means pulling in a lot of infrastructure.  But it's possible.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2025-08-19 13:37:50 +09:30
Rusty Russell
194febe873 wallet: generate fixup chainmoves and channelmoves when first starting.
If we don't have an accountdb from bookkeeper:

1. Generate a deposit chain event for every confirmed UTXO.
2. Generate an open chain event for every open, confirmed channel.
3. Generate a push/lease event if necessary.
4. Generate a fixup "journal" entry if balance is different from initial.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2025-08-19 13:37:50 +09:30
Rusty Russell
41ac9db7ea bookkeeper: remove database.
Changelog-Changed: Plugins: `bookkeeper` now uses the lightningd database, not "accounts.db".
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2025-08-19 13:37:50 +09:30
Rusty Russell
2a191479a3 bkpr: take, don't steal in new_channel_event.
Cleaner (I'm about to hand it a sha256 on the stack).

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2025-08-19 13:37:50 +09:30
Rusty Russell
45e860ad58 bkpr: helpers to query sql plugin for chainmoves and channelmoves.
We're going to be using this instead of our internal db.

I also made json_out_obj() take the str arg, as it didn't and I
expected it to.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2025-08-19 13:37:50 +09:30
Rusty Russell
9cc08bf06d plugins/bkpr/test/run-recorder: remove.
It's a great test, but it's very hard to simulate now we are going to be
going from the internal db.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2025-08-19 13:37:50 +09:30
Rusty Russell
c5e359c7ee bkpr: add in-mem & datastore storage for external blockheights.
We won't be able to "UPDATE chain_events", so keep a separate record
of these blockheights, and lookup that when the blockheight is 0.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2025-08-19 13:37:50 +09:30
Rusty Russell
e28443ed40 bkpr: pass bkpr struct into all functions in recorder.c.
We want to access it in stmt2chain_event, so plumb it through.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2025-08-19 13:37:50 +09:30
Rusty Russell
0afc4c0346 bookkeeper: use the in-memory rebalances.
Remove the rebalance field from channel_event, and use the
find_rebalance(bkpr, ev->db_id) to look it up instead.

chain_event's also had a `rebalance` field, but it was only ever set
(to false), never read.

Note: list_rebalances() was only used by tests, not a public API.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2025-08-19 13:37:50 +09:30
Rusty Russell
af2e403490 bkpr: use the in-memory event descriptions, not the database ones.
The new access APIs are more symmetrical:

1. edit_utxo_description -> add_utxo_description
2. add_payment_hash_desc -> add_payment_hash_description

And to read it, instead of accessing ->ev_desc (now removed) we use
chain_event_description() & channel_event_description(), threading bkpr though
as needed.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2025-08-19 13:37:50 +09:30
Rusty Russell
bf9c864e0f bkpr: save and restore onchain_fees from the datastore, not the db.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2025-08-19 13:37:50 +09:30
Rusty Russell
4e49a66c03 bkpr: keep onchain_fees in memory & datastore.
This is not quite as efficient, perhaps, but in practice there are only
a handful of onchain fee records per account.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2025-08-19 13:37:50 +09:30
Rusty Russell
568f12ea71 bkpr: remove redundant account_onchain_fees
It's account_get_chain_fees() without the ORDER BY clause, *and* it's
only used on the run-recorder.c test.

I also got ChatGPT to compare them, in case I missed something subtle.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2025-08-19 13:37:50 +09:30
Rusty Russell
5dc88c9e84 bkpr: move onchain fees access into bkpr/onchain_fee.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2025-08-19 13:37:50 +09:30
Rusty Russell
fd657e8483 ]bkpr: save accounts in the datastore instead of database.
Note that the test where we remove the database causes the bookkeeper
plugin to assert, since we have removed part (but not all!) of its data
by removing the datastore.

Once the transition to the datastore is complete, this can be restored.

Note that we destroy the request before receiving a response, which causes
a message in the trace span which was confusing our test.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2025-08-19 13:37:50 +09:30
Rusty Russell
9c786a20a7 bkpr: use direct names, not another table, in db.
This simplifies the transition (since accounts are going to be outside
the db).

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2025-08-19 13:37:50 +09:30
Rusty Russell
f72df6da9c bkpr: cache accounts table in memory.
When we don't have our own db, we're going to need to keep this information
in memory (and the datastore).  As a first step, simply cache it in memory
and still write through to the db.

This introduces some changes:
1. The account structures are not temporary, but in the hash table (so don't steal them).
2. test_forward_pad_fees_and_cltv assumed ordering, which was a latent bug.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2025-08-19 13:37:50 +09:30
Rusty Russell
fb20a55706 bkpr: find_or_create_account() instead of raw new_account().
In practice, it's always either find_account() or find_or_create_account().

This means account_add can be made internal: we rename it to
account_db_add.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2025-08-19 13:37:50 +09:30
Rusty Russell
40c0d86a5c bkpr: move account db functions from recorder.c into account.c.
This will let us cache them gradually in account.c, and eventually
remove the database table.

We also change find_close_account() to find_close_account_name(),
and maybe_mark_account_onchain() into account_onchain_closeheight()
and account_update_closeheight();

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2025-08-19 13:37:50 +09:30
Rusty Russell
b7e95e49b4 bkpr: send a backtrace if we get a fatal db event.
Particularly useful if we do something outside a transaction if I mess
up the code, e.g:

```
bookkeeper: Attempting to prepare a db_stmt outside of a transaction: plugins/bkpr/recorder.c:591 (version v25.05-112-g4daf355-modded)
0x5caefda2414b send_backtrace
	common/daemon.c:33
0x5caefda08a30 db_error
	plugins/bkpr/db.c:133
0x5caefda4cb32 db_fatal
	db/utils.c:322
0x5caefda4cc9e db_prepare_v2_
	db/utils.c:103
0x5caefda0dcbe get_chain_events_by_id
	plugins/bkpr/recorder.c:591
0x5caefda063ce json_edit_desc_payment_id
	plugins/bkpr/bookkeeper.c:550
0x5caefda13ad3 ld_command_handle
	plugins/libplugin.c:2185
0x5caefda13d27 ld_read_json_one
	plugins/libplugin.c:2274
0x5caefda13dba ld_read_json
	plugins/libplugin.c:2294
0x5caefdab5478 next_plan
	ccan/ccan/io/io.c:60
0x5caefdab5949 do_plan
	ccan/ccan/io/io.c:422
0x5caefdab5a06 io_ready
	ccan/ccan/io/io.c:439
0x5caefdab7375 io_loop
	ccan/ccan/io/poll.c:455
0x5caefda1420c plugin_main
	plugins/libplugin.c:2507
0x5caefda07762 main
	plugins/bkpr/bookkeeper.c:2048
0x79a7c522a1c9 __libc_start_call_main
	../sysdeps/nptl/libc_start_call_main.h:58
0x79a7c522a28a __libc_start_main_impl
	../csu/libc-start.c:360
0x5caefda00374 ???
	???:0
0xffffffffffffffff ???
	???:0
Attempting to prepare a db_stmt outside of a transaction: plugins/bkpr/recorder.c:591
```
2025-08-19 13:37:50 +09:30
Rusty Russell
22b452ad19 lightningd: have onchaind inform us when to make a channel penalty_adj.
bookkeeper used to generate these as channel events, now lightningd does.

We also add a "journal" event, which we will need later too.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2025-08-14 19:49:09 +09:30
Rusty Russell
b1fa2ef30b bkpr: remove different currency support.
We still output the fields, they're just always the currency of the node.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Changed: Plugins: `bookkeeper` now explicitly assumes every transaction is in the same currency as the node (true unless you added manually)
2025-08-14 19:49:09 +09:30
Rusty Russell
47c2735e17 bkpr: remove currency support from balances.
We're going to get rid of this concept, but the main change is that the
account_get_balance API can be drastically simplified:

account_get_credit_debit() accesses the raw fields, never fails, but
returns the a flag which tells us if the account doesn't actually have
any events.

The one place we care about the balance, calculate by hand.  Then
account_get_balance() (and struct account_balance) can simply be
moved to th test.

Subtly, without the "GROUP BY" clause, you always get one row, even if
there are no rows (but the SUM are null).

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2025-08-14 19:49:09 +09:30
Rusty Russell
0c9dc4cf07 bkpr: remove unused "account_exits" parameter to account_get_balance().
Only used in tests.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2025-08-14 19:49:09 +09:30
Rusty Russell
b0231a59d8 common: put "external" and "wallet" strings, and test functions into common/coin_mvt.h
They're scattered and reproduced in many places: unify them.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2025-08-14 19:49:09 +09:30
Rusty Russell
5756b54f38 common: rename enum mvt_tag values.
Prefix MVT_ to them, for clarity.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2025-08-14 19:49:09 +09:30
Rusty Russell
6fc0b1f3d1 common/coin_mvt: add struct mvt_account_id to separate channels from others.
This means we can keep a pointer to the channel directly, *or* a string.
This avoids gratuitous formatting (on creation) and lookups (later).

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2025-08-14 19:49:09 +09:30
Rusty Russell
0534c524b5 common: remove "ignored" tag.
We don't actually set it any more.  The bookkeeper db does a migration
for old anchors.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2025-08-14 19:49:09 +09:30
Matt Whitlock
9112c1d518 common/json_parse_simple: make convenience functions inline
json_tok_streq(…) and json_get_member(…) are convenience wrappers for
json_tok_strneq(…) and json_get_membern(…) respectively. Unfortunately, using
them incurs a performance penalty in the common case where they are called with
a string literal argument because the compiler is unable to substitute a
compile-time constant in place of the buried call to strlen(…).

For example,

	json_get_member(buf, tok, "example");

…will have worse performance than…

	json_get_membern(buf, tok, "example", strlen("example"));

…because the former is forced to scan over "example" at run-time to count its
length whereas the latter is able to elide the strlen(…) call at compile time.

Hoist these convenience functions up into common/json_parse_simple.h and mark
them as inline so that the compiler can elide the strlen(…) call in the common
case of calling these functions with a string literal argument.

Changelog-None
2025-08-14 17:53:39 +09:30
Rusty Russell
5a3943f2e0 bkpr: fix unittest when HAVE_USDT=1
```
pseudorand_u64 called!
Aborted (core dumped)
make: *** [Makefile:786: unittest/plugins/bkpr/test/run-bkpr_db] Error 134
```

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2025-04-24 17:34:12 +09:30
Rusty Russell
6a8e586ea8 trace: don't use randombytes_buf(), use pseudorand.
This is much faster to give 64 bits of data, and we don't need
cryptographic randomness.

This brings us back to 413ns per trace.

Before:
	real	0m5.819000-6.472000(6.2064+/-0.26)s
	user	0m3.779000-4.101000(3.956+/-0.12)s
	sys	0m2.040000-2.431000(2.2496+/-0.15)s

After:
	real	0m3.981000-4.247000(4.1276+/-0.11)s
	user	0m3.979000-4.245000(4.126+/-0.11)s
	sys	0m0.000000-0.002000(0.001+/-0.00063)s

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Fixed: lightingd: trimmed overhead of tracing infrastructure.
2025-04-23 13:45:18 +09:30
Alex Myers
c9b01b60a0 unit-tests: cleanup traces after testing
Otherwise it appears to be a leak:

==612637== 11,264 bytes in 1 blocks are still reachable in loss record 1 of 1
==612637==    at 0x484D953: calloc (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==612637==    by 0x1301F2: trace_init (trace.c:153)
==612637==    by 0x13065D: trace_span_start (trace.c:263)
==612637==    by 0x173968: db_open_ (utils.c:367)
==612637==    by 0x17AE43: create_test_wallet (run-wallet.c:1313)
==612637==    by 0x17C726: test_shachain_crud (run-wallet.c:1548)
==612637==    by 0x18300E: main (run-wallet.c:2329)

Changelog-None
2025-02-11 19:16:16 -06:00
Rusty Russell
68feb55dbf wallet: save last known address.
If we connected out, remember that address.  We always remember the last
address, but that may be an incoming address.  This is explicitly the last
outgoing address which worked.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2024-11-25 15:39:13 +10:30
Christian Decker
04b4f61308 db: Instrument the DB interactions to trace their execution 2024-11-18 17:46:40 +01:00
Christian Decker
f36be4b006 plugin: Add tracing support for send_outreq
The `send_outreq` function is a good place to suspend and resume
traces, since these are usually the places where we hand off control
back to the `io_loop`. This assumes that we do not continue doing
heavy liftin after we have queued an `outreq` call, but that is most
likely the case anyway. This frees us from having to track suspensions
whenever we call the RPC from a plugin.
2024-11-13 13:15:52 +01:00
Rusty Russell
679f46f733 common/amount: rename amount_sat_zero/amount_msat_zerp -> amount_sat_is_zero/amount_msat_is_zero.
I used `amount_msat_eq(x, AMOUNT_MSAT(0))` because I forgot this
function existed.  I probably missed it because the name is surprising,
so add "is" in there to make it clear it's a boolean function.

You'll note almost all the places which did use it are Eduardo's and
Lisa's code, so maybe it's just me.

Fix up a few places which I could use it, too.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2024-09-19 12:16:53 +09:30
niftynei
46dcc6e031 bkpr: properly account for fees and channel closures if splice
We do some fancy accounting for channel closures; since we're tagging
splice txs as closes we need to mark them as splices so we can treat them
as any other 'normal' on chain event.
2024-08-08 12:30:53 -07:00
Rusty Russell
9fd29d35dd common: move now-enlarged command_fail_badparam into its own source file.
It's getting a bit awkward to inline now: it's non-trivial.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2024-06-25 08:47:50 +09:30
Alex Myers
d30b1c8dc2 unit-test: update mocks 2024-05-20 16:58:49 -05:00
Vincenzo Palazzo
ce70167ead core: notify plugins when a log line is emitted.
Currently make a plugin that do reportings of logs on
a services like graphana is not possible. So this commit
include the possibility to write a plugin that do the report
of this analisys.

Changelog-Added: core: notify plugins when a log line is emitted.
Signed-off-by: Vincenzo Palazzo <vincenzopalazzodev@gmail.com>
2024-05-17 13:03:12 -05:00
Rusty Russell
fc21fc56dc libplugin: handle check for our commands.
We don't thoroughly handle `check setconfig`: it would be good to
allow this to do further checking!

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2024-05-06 20:51:19 -05:00
Rusty Russell
e0e879c003 common: remove type_to_string files altogther.
This means including <common/utils.h> where it was indirectly included.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2024-03-20 13:51:48 +10:30
Rusty Russell
37d22f9141 global: change all type_to_string to fmt_X.
This has the benefit of being shorter, as well as more reliable (you
will get a link error if we can't print it, not a runtime one!).

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2024-03-20 13:51:48 +10:30
niftynei
a4f92eac81 bkpr: now that we're not doing empty acct logging, we dont need this bool
We stopped doing empty journal logs, so we no longer need to switch
our log severity based on whether or not an account exists.

Should make bookkeeper less chatty and remove noisy logs

Changelog-None
2024-01-29 10:05:03 +10:30
Rusty Russell
8e6eaf2511 common: allow JSON-RPC parameters to specify deprecation versions.
This infrastructure is use by both libplugin and lightningd.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2024-01-26 10:30:22 +10:30
Rusty Russell
7431b8b436 common: add command_deprecated_param_ok() and command_deprecated_out_ok()
Generic helpers for libplugin and lightningd.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2024-01-26 10:30:22 +10:30
Rusty Russell
1ecf31bae7 db: remove #if DEVELOPER in favor of runtime flag inside db struct.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2023-09-21 20:08:24 +09:30
Rusty Russell
a9f26b7d07 common/daemon.c: remove #ifdef DEVELOPER in favor of runtime flag.
Also requires us to expose memleak when !DEVELOPER, however we only
ever used the memleak tracking when the LIGHTNINGD_DEV_MEMLEAK
environment variable was set, so keep that.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2023-09-21 20:08:24 +09:30
Rusty Russell
04f485aee0 db: add generic warn/error function.
This avoids the mess where we override db_fatal for teqsts, and keeps it
generic.

Also allows us to get rid of one #if DEVELOPER, and an ugly global for
bookkeeper.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2023-07-09 16:49:48 +09:30