Add deduplication or stable IDs for user-facing events

[joostjager]

Problem

ldk-node exposes user-facing events through EventQueue in src/event.rs. The queue is separate from LDK's own event persistence and acknowledgement. LDK events are at-least-once, so ldk-node may see the same underlying LDK event again after restart if that LDK event was not durably recorded as handled.

EventQueue::add_event currently:

1. Locks the in-memory VecDeque<Event>.
2. Pushes the new Event with push_back.
3. Serializes the entire queue with EventQueueSerWrapper.
4. Persists the queue with KVStore::write under the event queue persistence key.

There is no stable app-event ID, no event fingerprint, and no queue-level deduplication. Node::next_event and Node::next_event_async return the front of this queue, and Node::event_handled removes the front event and persists the shorter queue. That API makes delivery persistent until the application acknowledges an event, but it does not protect against duplicate events that were appended because LDK replayed the underlying event.

This is not specific to payments. Any user-facing event derived from a replayed LDK event can be appended again if the ldk-node event was already persisted but the underlying LDK event was not yet durably recorded as handled.

Payment events do carry useful dedupe material. Current PaymentSuccessful, PaymentFailed, and PaymentReceived events are emitted with payment_id: Some(...), and they also carry payment hashes where available. That means applications that already treat payment events as idempotent by payment ID or payment hash can avoid the worst outcomes. The remaining gap is that ldk-node can still deliver duplicate app events, the API does not expose a stable event ID, and the idempotency contract is implicit rather than documented or enforced by the queue. There are also compatibility and edge-case wrinkles: older serialized events can have payment_id: None, PaymentFailed can have payment_hash: None for some BOLT12 failures, and current-main inbound PaymentReceived uses a hash-derived PaymentId rather than the LDK event payment_id.

Consequence

Applications may receive duplicate user-facing events. Payment events include identifiers that let careful applications dedupe most payment-side effects, so the channel-fund-safety risk is not the concern here. The risk is API-level ambiguity: applications that assume exactly-once event delivery may create duplicate accounting entries, notifications, fulfillment actions, or operational work. For non-payment events, the consequence depends on what the application does in response and whether the event exposes enough stable fields to dedupe reliably.

Possible Fix

ldk-node could expose stable event IDs, deduplicate queued events using an event-specific fingerprint, or document and enforce an app-facing idempotency contract. If the queue can recognize that a replayed LDK event would produce the same user-facing event again, it can avoid appending a duplicate PaymentSuccessful, PaymentFailed, or PaymentReceived event. That would address the duplicate outbound and inbound payment-event cases without changing LDK's at-least-once event delivery model.

[joostjager]

Thinking this issue should probably also cover the persistence ordering inside EventQueue itself.

Currently EventQueue::add_event and EventQueue::event_handled mutate the in-memory queue before writing the serialized queue to the store. If the store write fails, the durable queue and in-memory queue can diverge: add_event can leave an event visible in the current process even though it was not durably queued, while event_handled can remove an event from memory even though it remains in the durable queue and may reappear after restart.

So besides stable IDs / dedupe for replayed LDK events, there is a simpler queue consistency question here too: should EventQueue write the prospective queue first and only mutate the live in-memory queue after persistence succeeds?

[tnull]

We discussed this at some point upstream, and I think if we want to do this we should add idempotency tokens in the LDK events first.

@joostjager Are you fine with moving the issue there?

[joostjager]

Make sense. Will move the issue there. The question about queue consistency in ldk-node (write before in-memory mutate) is one thing that remains?

[tnull]

Make sense. Will move the issue there. The question about queue consistency in ldk-node (write before in-memory mutate) is one thing that remains?

I guess we could switch to that model (recently did the same for DataStore). To do this efficiently we might need to switch the event queue mutex over to be a tokio::sync::Mutex, which in turn requires any methods accessing the queue to be async, would need to explore if that works out.