Add persistent closed channel history and `list_closed_channels()` by f3r10 · Pull Request #882 · lightningdevkit/ldk-node

f3r10 · 2026-04-20T16:31:30Z

Summary

Closes #851.

Adds persistent historical closed channel records so applications can query all channels that have ever closed on this node, surviving restarts.

New public API

Node::list_closed_channels() -> Vec<ClosedChannelDetails>
ClosedChannelDetails struct: channel_id, user_channel_id,
counterparty_node_id, funding_txo, channel_capacity_sats,
last_local_balance_msat, is_outbound, closure_reason, closed_at
Event::ChannelClosed gains three new fields: channel_capacity_sats,
channel_funding_txo, last_local_balance_msat

Implementation

Persistence (src/closed_channel.rs, src/io/): ClosedChannelDetails implements StorableObject with insert-only semantics (closed records are immutable). Records are stored under the "closed_channels" KV namespace keyed by UserChannelId.

Startup (src/builder.rs): read_closed_channels() joins the existing tokio::join! block so historical records are loaded in parallel with payments, pending payments, and node metrics — no added sequential startup cost as history gronws.

Event handlinng (src/event.rs): On ChannelClosed, the handler writes a ClosedChannelDetails record to the store and emits the enriched public event. Channel direction (is_outbound) is tracked via an in-memory outbound_channel_ids set, seeded from channel_manager.list_channels() at startup and updated on ChannelPending, since ChannelClosed does not carry that information directly.

ldk-reviews-bot · 2026-04-20T16:31:32Z

👋 Thanks for assigning @tnull as a reviewer!
I'll wait for their review and will help manage the review process.
Once they submit their review, I'll check if a second reviewer would be helpful.

Camillarhi

Thanks for looking into this. I dropped some comments. Also, CI seems to be failing due to a formatting issue. Kindly run cargo fmt --all to resolve this

Camillarhi · 2026-04-20T20:28:51Z

+			closed_at: closed_at.0.ok_or(DecodeError::InvalidValue)?,
+		})
+	}
+}


Can we use impl_writeable_tlv_based! here instead? Matches the rest of the StorableObject types in the codebase

Camillarhi · 2026-04-20T20:42:26Z

+	/// Retrieve a list of closed channels.
+	///
+	/// Channels are added to this list when they are closed and will be persisted across restarts.
+	pub fn list_closed_channels(&self) -> Vec<ClosedChannelDetails> {


The new list_closed_channels() API isn't exposed in bindings.

Camillarhi · 2026-04-20T20:45:33Z

+/// [`Node::list_closed_channels`]: crate::Node::list_closed_channels
+#[derive(Clone, Debug, PartialEq, Eq)]
+#[cfg_attr(feature = "uniffi", derive(uniffi::Record))]
+pub struct ClosedChannelDetails {


ClosedChannelDetails isn't exposed in bindings

ldk-reviews-bot · 2026-04-22T16:42:32Z

🔔 1st Reminder

Hey @tnull! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

ldk-reviews-bot · 2026-04-25T00:00:41Z

🔔 2nd Reminder

Hey @tnull! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

ldk-reviews-bot · 2026-04-27T00:01:50Z

🔔 3rd Reminder

Hey @tnull! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

tnull

Thanks, already looks pretty good. Some comments.

tnull · 2026-04-27T11:18:05Z

+	let mut set = tokio::task::JoinSet::new();
+
+	// Fill JoinSet with tasks if possible
+	while set.len() < BATCH_SIZE && !stored_keys.is_empty() {


Makes sense to DRY this up for now, but depending on when #876 lands we might need to adapt this to use the new BatchingStore.

rebased to use the latest read_all_objects

tnull · 2026-04-27T11:25:34Z

+		assert_eq!(record.funding_txo.unwrap().txid, funding_txo.txid);
+		assert!(record.closure_reason.is_some());
+
+		closed_channel_before = record.clone();


nit: Why do we clone here?

tnull · 2026-04-27T11:26:12Z

+		node_b.sync_wallets().unwrap();
+
+		// Verify the record is present before restart.
+		let closed_a = node_a.list_closed_channels();


Should we also check that b also persisted the closed channel?

tnull · 2026-04-27T11:36:46Z

I think we should be good breaking backwards compat with LDK node v0.1, so let's make this a required field. Then we can also avoid having the Option propagate. Please add a not regarding serialization compatibility as part of a # Pending section in CHANGELOG.md

tnull · 2026-04-27T11:37:33Z

+/// [`Node::list_closed_channels`]: crate::Node::list_closed_channels
+#[derive(Clone, Debug, PartialEq, Eq)]
+#[cfg_attr(feature = "uniffi", derive(uniffi::Record))]
+pub struct ClosedChannelDetails {


Might be good also recording whether this was an announced channel.

Introduce `ClosedChannelDetails`, a new record type persisted to the KV store under the `"closed_channels"` namespace whenever a channel closes. Records are written in the `ChannelClosed` event handler and loaded back at startup in parallel with other stores via `tokio::join!`. Add `Node::list_closed_channels()` to expose the full history of closed channels across restarts. Track outbound channel direction via an in-memory `outbound_channel_ids` set seeded from `channel_manager.list_channels()` at startup and updated on `ChannelPending` events, since `ChannelClosed` does not carry that information directly.

…sed event We no longer need to maintain backwards compatibility with LDK Node v0.1.0, so we can make this a required field and avoid propagating the Option through the API.

Camillarhi

Thanks! I left a few more comments.

Camillarhi · 2026-05-15T15:35:23Z

+	/// This will be `false` for channels opened prior to this field being tracked.
+	pub is_announced: bool,
+	/// The reason for the channel closure.
+	pub closure_reason: Option<ClosureReason>,


Is there a reason closure_reason is Option<ClosureReason>? The LDK reason on ChannelClosed isn't optional and we always wrap it in Some at write time, so I don't think we can ever persist a None. Could it just be ClosureReason?

This seems valid. More generally, for all optional fields here, please add a note to the docs describing when/from which version they can be expected to be set.

As I understand, ClosureReason is an evolving enum — LDK could add new variants over time.
If we used required + a plain ClosureReason field, an unknown future variant would cause read_all_objects to return an error, and the node would fail to start 🤔
wdyt @Camillarhi ?

As I understand, ClosureReason is an evolving enum — LDK could add new variants over time. If we used required + a plain ClosureReason field, an unknown future variant would cause read_all_objects to return an error, and the node would fail to start 🤔 wdyt @Camillarhi ?

Hmm, no, LDK's compatibility guarantees will protect us from that. Upstream we'd add new fields only as optional/odd usually, to ensure we never break serialization compatibility.

oh ok thanks @Camillarhi. I am going to remove the Option there

@Camillarhi I tried to make the change, but it looks like ClosureReason intentionally does not implement Readable only MaybeReadable, and impl_writeable_tlv_based! returns an error 🤔

Camillarhi · 2026-05-15T15:44:07Z

+					counterparty_node_id: counterparty_node_id
+						.expect("counterparty_node_id must be set for closed channels"),


This could panic as the counterparty_node_id from LDK is Option<Publickey>

ldk-reviews-bot · 2026-05-16T00:00:58Z

🔔 1st Reminder

Hey @tnull! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

ldk-reviews-bot · 2026-05-18T00:02:00Z

🔔 2nd Reminder

Hey @tnull! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

ldk-reviews-bot · 2026-05-20T00:02:28Z

🔔 3rd Reminder

Hey @tnull! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

tnull

Feel free to squash. Two minor comments, and then we should move forward with this.

tnull · 2026-05-21T15:04:24Z

 					user_channel_id,
-					counterparty_node_id,
+					counterparty_node_id: counterparty_node_id
+						.expect("counterparty_node_id must be set for closed channels"),


Please provide a reason why this expectation always holds (i.e. since which version of LDK or LDK node this is the case).

@tnull I added a changelog entry about this change https://github.com/lightningdevkit/ldk-node/pull/882/changes#diff-06572a96a58dc510037d5efa622f9bec8519bc1beab13c9f251e97e657a9d4edR3
The `counterparty_node_id` field of the `ChannelReady` and `ChannelClosed` events is now required. Events persisted by LDK Node v0.1.0 and prior that are missing this field will fail to deserialize.
But since LDK can return None for early-lifecycle close, maybe the right approach would be: revert Event::ChannelClosed::counterparty_node_id back to Option, remove the .expect(), and remove the CHANGELOG entry. Should I make those changes?

No, breaking backwards compat with LDK Node v0.1 is fine, we should just (very briefly) also mention in the except that that is what's happening (i.e., that >v0.1 this is expected to hold true).

Great, I am going to add that note

tnull · 2026-05-21T15:14:48Z

+	/// This will be `false` for channels opened prior to this field being tracked.
+	pub is_announced: bool,
+	/// The reason for the channel closure.
+	pub closure_reason: Option<ClosureReason>,


This seems valid. More generally, for all optional fields here, please add a note to the docs describing when/from which version they can be expected to be set.

tnull · 2026-05-21T15:34:40Z


-				let event = Event::ChannelClosed {
+				let user_channel_id = UserChannelId(user_channel_id);
+				let is_outbound = self


Codex:

[P2] Persist channel flags for replayed closures — /home/tnull/workspace/ldk-node/src/event.rs:1646-1655
When a ChannelClosed LDK event is replayed after a restart or after handle_event returns ReplayEvent, the channel can already be gone from list_channels() and these process-local sets are not reconstructed. In that case .remove() returns false, so an outbound/public channel is persisted in
ClosedChannelDetails as inbound/unannounced; a failed first attempt also consumes the entries before replay. Store these flags durably before closure or make this path idempotent.

tnull · 2026-05-21T15:35:04Z

+					closed_at,
+				};
+
+				if let Err(e) = self.closed_channel_store.insert(record) {


Codex:

[P2] Replay ChannelClosed when history persistence fails — /home/tnull/workspace/ldk-node/src/event.rs:1678-1685
If closed_channel_store.insert(record) fails but event_queue.add_event succeeds, this only logs the error and eventually returns Ok(()), so LDK considers the ChannelClosed handled and will not replay it. The user still sees the close event, but list_closed_channels() permanently misses the
record; propagate this failure as ReplayEvent like other persisted event side effects.

Fall back to the already-persisted record for is_outbound/is_announced when in-memory sets are empty on replay, use insert_or_update to avoid overwriting correct values, and propagate persist failures as ReplayEvent.

ldk-reviews-bot requested a review from tnull April 20, 2026 16:42

Camillarhi suggested changes Apr 20, 2026

View reviewed changes

tnull reviewed Apr 27, 2026

View reviewed changes

f3r10 force-pushed the feat/persist_historical_channels branch 3 times, most recently from ac11095 to 20b75bb Compare May 13, 2026 18:55

f3r10 requested review from Camillarhi and tnull May 13, 2026 18:55

f3r10 added 2 commits May 13, 2026 14:43

Make counterparty_node_id non-optional on ChannelReady and ChannelClo…

1960f50

…sed event We no longer need to maintain backwards compatibility with LDK Node v0.1.0, so we can make this a required field and avoid propagating the Option through the API.

f3r10 force-pushed the feat/persist_historical_channels branch from 20b75bb to 1960f50 Compare May 13, 2026 19:43

Camillarhi suggested changes May 15, 2026

View reviewed changes

jkczyz mentioned this pull request May 21, 2026

Add splice RBF support #888

Open

tnull reviewed May 21, 2026

View reviewed changes

f3r10 added 2 commits May 22, 2026 11:16

Make ChannelClosed persistence idempotent and retry on failure

583b2b0

Fall back to the already-persisted record for is_outbound/is_announced when in-memory sets are empty on replay, use insert_or_update to avoid overwriting correct values, and propagate persist failures as ReplayEvent.

Document when optional fields of ClosedChannelDetails can be None

afa2c7b

f3r10 requested a review from tnull May 22, 2026 16:19

		counterparty_node_id: counterparty_node_id
		.expect("counterparty_node_id must be set for closed channels"),

Conversation

f3r10 commented Apr 20, 2026

Summary

New public API

Implementation

Uh oh!

ldk-reviews-bot commented Apr 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Camillarhi left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ldk-reviews-bot commented Apr 22, 2026

Uh oh!

ldk-reviews-bot commented Apr 25, 2026

Uh oh!

ldk-reviews-bot commented Apr 27, 2026

Uh oh!

tnull left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Camillarhi left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ldk-reviews-bot commented May 16, 2026

Uh oh!

ldk-reviews-bot commented May 18, 2026

Uh oh!

ldk-reviews-bot commented May 20, 2026

Uh oh!

tnull left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

f3r10 May 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tnull May 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

ldk-reviews-bot commented Apr 20, 2026 •

edited

Loading

f3r10 May 21, 2026 •

edited

Loading

tnull May 22, 2026 •

edited

Loading