Add atomic KVStore batch writes for restart-consistent state

[joostjager]

Issue #966 tracks a recurring class of crash/replay gaps between LDK-managed state and ldk-node-managed state. LDK persists ChannelManager, ChannelMonitors, and replayable LDK events. ldk-node separately persists PaymentDetails in PaymentStore and user-facing events in EventQueue.

When those streams are written independently, one side of a logical transition can become durable while the other does not. That leads to missing payment records, replay-only reconstruction, duplicate app events, startup backfill questions, and case-by-case reconciliation logic.

I think we should consider adding an atomic batch-write primitive to the ldk-node KV store layer, so related records can be committed as one restart-consistent unit. For example, one batch could include the LDK-derived persistence update, the corresponding PaymentStore update, and the EventQueue update that exposes it to the application.

The goal is not to require SQL, one big serialized blob, or a full LDK persistence redesign. The narrower goal is to let ldk-node write semantically coupled records together when needed, and have either all of them become durable or none of them become durable.

This seems like a natural extension of deferred writing. We already hold work until a persistence boundary; the missing piece is that ldk-node records still land through separate streams. If the underlying store supports atomic batches, the same boundary could include ldk-node state too.

[tnull]

This one should def. live / get discussed further on LDK (first)?

[joostjager]

I’d actually push back on starting this in LDK. We’ve discussed an LDK-level atomic batch write a few times before, and the blocker has always been that it needs a credible story for all supported KVStore implementations, especially FilesystemStore. I don’t think we have consensus on how to offer true multi-key atomicity there.

So I think that if we want to make progress, this should start in ldk-node instead. The consistency problem here is specifically between LDK-managed writes and ldk-node-managed writes for payments and events.

The direction I’d be thinking of is a StagingStore wrapper that ldk-node passes to LDK. That store would collect LDK write/remove operations and keep their futures pending. Then ldk-node would have a coordinator point where it atomically commits the staged LDK writes plus the corresponding ldk-node writes to a real backing store that supports this. Only after that commit succeeds would the staged LDK futures resolve.

Not ideal, but that may be the price of maintaining flexibility.

[tnull]

Then ldk-node would have a coordinator point where it atomically commits the staged LDK writes plus the corresponding ldk-node writes to a real backing store that supports this. Only after that commit succeeds would the staged LDK futures resolve.

I don't know how that would work if the underlying stores don't support atomic batch writes?

[joostjager]

It wouldn’t work with backing stores that don’t support atomic batch writes.

The StagingStore would not manufacture atomicity where the underlying storage layer doesn’t have it. It would only give ldk-node a coordination point: keep LDK writes pending, then resolve them once ldk-node has committed them together with the related payment/event writes.

So the stronger restart-consistency guarantee would only apply when the selected ldk-node backing store can actually commit that batch atomically.

[tnull]

It wouldn’t work with backing stores that don’t support atomic batch writes.

Right, but even for stores that technically do, we can't access that internally if the stores (presumably living in lightning-persister) don't expose that functionality via the API.

[joostjager]

Then I’d say: don’t put that capability behind that boundary.

The split between ldk-node, lightning-persister, and the store implementation is somewhat arbitrary. It shouldn’t limit functionality, and especially not robustness. If we need atomic commits to avoid split-state failures, ldk-node should have access to a backing-store API that can provide them.

[tnull]

The split between ldk-node, lightning-persister, and the store implementation is somewhat arbitrary.

It's not. It was a temporary workaround that we keep paying the price for over and over again. It was a mistake at a time and we need to finally fix it by consolidating all implementations in a single place.

[joostjager]

Maybe “arbitrary” was the wrong word. What I mean is that these boundaries are still something we introduced ourselves.

If we keep that boundary in place, we also force ldk-node to keep multiple persistence layers that cannot be committed together. The consequence is hard-to-see split-state failures, plus ongoing cost in developer time whenever we need to reason across those layers.

And so far, an atomic batch primitive in LDK itself has been a no-go. So I think we should be explicit that this structural preference has both a robustness cost and a maintenance cost.