Skip to content

Conversation

rustyrussell
Copy link
Contributor

All the issues I found while trying to make deterministic core lightning runs.

  1. A crash when we splice a confirmed-but-unannounced channel.
  2. listtransactions not noticing when a sendpsbt transaction gets mined, if there are no outputs to us.
  3. Improve robustness of notleak() macro so we can use it very early.
  4. Make --dev-save-plugin-io more robust against restarts, by including timestamp in filename.
  5. Remove unused functions and fields.
  6. Fix benchmark routines to use timemono not timeabs.
  7. Fix invalid output in scb files (to do with last seen address).

continue;

if (!inflight->funding->splice_remote_funding)
continue;
Copy link
Collaborator

@ddustin ddustin Sep 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps this should be an assert or channel fail. In the past we didn't allow the changing of remote funding pubkeys -- so this should only come up if you migrated up versions with a pending splice hanging in the inflights.

If it happens in a normal context it should be some kind of internal error -- splice_remote_funding is required now (comes from splice or splice_ack message).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It crashed when generating examples, but I couldn't reproduce. I will get upset here...

@ddustin
Copy link
Collaborator

ddustin commented Sep 19, 2025

Do you have more the log handy for this crash? 9d76481

@rustyrussell
Copy link
Contributor Author

Do you have more the log handy for this crash? 9d76481

You can reproduce it yourself on master using this test. However, it was pretty clear, and the next commit fixes it...

```
DEBUG   lightningd: Got depth change 2->3 for e9e31956f77c3844ee2e6e4607dbfebdee95a9aa549668a7a429b8246a6a29de
**BROKEN** lightningd: FATAL SIGNAL 6 (version v25.09-20-g003ba4a)
**BROKEN** lightningd: backtrace: common/daemon.c:41 (send_backtrace) 0x619bef20e274
**BROKEN** lightningd: backtrace: common/daemon.c:78 (crashdump) 0x619bef20e408
**BROKEN** lightningd: backtrace: ./signal/../sysdeps/unix/sysv/linux/x86_64/libc_sigaction.c:0 ((null)) 0x7a1ccf24532f
**BROKEN** lightningd: backtrace: ./nptl/pthread_kill.c:44 (__pthread_kill_implementation) 0x7a1ccf29eb2c
**BROKEN** lightningd: backtrace: ./nptl/pthread_kill.c:78 (__pthread_kill_internal) 0x7a1ccf29eb2c
**BROKEN** lightningd: backtrace: ./nptl/pthread_kill.c:89 (__GI___pthread_kill) 0x7a1ccf29eb2c
**BROKEN** lightningd: backtrace: ../sysdeps/posix/raise.c:26 (__GI_raise) 0x7a1ccf24527d
**BROKEN** lightningd: backtrace: ./stdlib/abort.c:79 (__GI_abort) 0x7a1ccf2288fe
**BROKEN** lightningd: backtrace: ./assert/assert.c:96 (__assert_fail_base) 0x7a1ccf22881a
**BROKEN** lightningd: backtrace: ./assert/assert.c:105 (__assert_fail) 0x7a1ccf23b516
**BROKEN** lightningd: backtrace: lightningd/peer_control.c:2202 (funding_depth_cb) 0x619bef1ac497
**BROKEN** lightningd: backtrace: lightningd/watch.c:223 (txw_fire) 0x619bef1cfcbf
**BROKEN** lightningd: backtrace: lightningd/watch.c:292 (watch_topology_changed) 0x619bef1cffa4
**BROKEN** lightningd: backtrace: lightningd/chaintopology.c:829 (updates_complete) 0x619bef144a8c
**BROKEN** lightningd: backtrace: lightningd/chaintopology.c:1047 (get_new_block) 0x619bef14561e
```

Signed-off-by: Rusty Russell <[email protected]>
…w one via splice.

This happens if the channel is *not* announcable yet.  Then we hit the assertion
in funding_depth_cb that the txid is the same as the current funding.txid.

Signed-off-by: Rusty Russell <[email protected]>
Changelog-EXPERIMENTAL: fixed crash when we splice a channel which hasn't been announced yet.
I got a NULL deref on `infcopy->remote_funding = *inflight->funding->splice_remote_funding`
at once point in testing, so this should prevent that from happening,
yet still allow us to catch it in CI if it happens again.

Signed-off-by: Rusty Russell <[email protected]>
We watch if they are to do with a channel, or have outputs going to us, but otherwise
we didn't, so we never updated the blockheight in the db.

Signed-off-by: Rusty Russell <[email protected]>
Changelog-Fixed: JSON-RPC: `listtransactions` now correctly updates `blockheight` for txs created by `sendpsbt` which have no change outputs.
It now simply renames tal names, so it's harmless to do even if we're
not going to do memleak detection.

Signed-off-by: Rusty Russell <[email protected]>
@rustyrussell rustyrussell force-pushed the misc-fixes-for-examples branch from 66cfba5 to 7954cda Compare October 2, 2025 01:48
…eak coverage for any typed hash table.

You can now simply add per-tal-object helpers for memleak, but our older pattern required
calling memleak functions explicitly during memleak handling.  Hash tables in particular need
to be dynamically allocated (we override the allocators using htable_set_allocator and assume
this), so it makes sense to have a helper macro that does all three.

This eliminates a huge amount of code.

Signed-off-by: Rusty Russell <[email protected]>
…ames.

Incorporate a time: this covers the restart case as well.  And make it time_mono(),
which doesn't get overridden when we override normal wall time.

Signed-off-by: Rusty Russell <[email protected]>
I noticed, because it pulled in randomness routines.

Signed-off-by: Rusty Russell <[email protected]>
This was changing all the time when I tried to make
autogenerate-rpc-examples.py reproducible.  Turns out it was being
corrupted (it does suspicious things with pointers); rather than try
to diagnose it, I simply rewrote the code to create it only when we
need it.

Signed-off-by: Rusty Russell <[email protected]>
@rustyrussell rustyrussell force-pushed the misc-fixes-for-examples branch from 7954cda to c217e3a Compare October 2, 2025 02:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants