Fix EventQueue losing state on persistence failures

[elnafateh]

Fixes #1027.

Problem

EventQueue::add_event and EventQueue::event_handled mutate the in-memory queue before the mutation is durably persisted. If the persist call fails or races with another operation, in-memory and persisted state can diverge, causing:

1. Duplicate events — a failed add_event still leaves the event in memory; when LDK retries via ReplayEvent, the event gets enqueued a second time.
2. Silently skipped events — a failed event_handled still pops the front event from memory; a caller retrying the ack (per the documented "same event until confirmed" contract) actually acks the next event instead.
3. Restart resurrection — a mutation that's applied in memory but not yet persisted is lost on crash/restart, so an already-acked event can reappear.
4. Lost newer writes — without ordering, a stale in-flight persist can overwrite a newer one.

Fix

Serialize add_event/event_handled end-to-end behind a tokio::sync::Mutex (operation_lock) spanning snapshot-compute → persist → in-memory-commit. In-memory state is now only ever advanced after its snapshot is durably persisted, and persistence submissions happen strictly in mutation order.

This closes all four failure modes structurally:

• (1)/(2): a failed persist means the in-memory mutation never happens, so retries repeat the same logical operation instead of duplicating/skipping.

• (3)/(4): full serialization means there's no concurrent-write-submission window left to race — persisted state can never fall behind or get reordered relative to in-memory state.

I verified that SqliteStore, VssStore, and PostgresStore all allocate their write version synchronously at the top of write(), before any async work — which is what makes serializing at this layer sufficient to also close the built-in stores' residual version-ordering window. Custom KVStore implementations that defer version allocation to a later async step wouldn't get that same guarantee from this fix alone — worth flagging as a documented expectation of KVStore implementors if that's not already stated.

Testing

Added a fault-injecting KVStore wrapper (FaultingStore) that can be told to fail the next N writes, with regression tests for:

• Failed-enqueue replay (no duplicate)

• Failed-ack retry (no skip)

• Concurrent enqueue/ack under load (no lost updates, persisted state matches in-memory)

• Restart resurrection (persisted bytes never reflect an uncommitted mutation)

[ldk-reviews-bot]

I've assigned @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.

[Jolah1]

Nice work!
The production approach looks correct: the operation mutex spans snapshot creation, persistence completion, and the in-memory commit. This fixes failed enqueue/acknowledgement behavior and serializes successful queue mutation.

The two ordering-related tests do not regress the ordering failures described in #1027. The concurrency test uses immediately completing writes and only checks that 200 adds plus 200 acknowledgements leave an empty queue. The restart test checks restoration after a failed acknowledgement, and its final assertion would also pass against the old implementation.

you should add a deterministic store that can hold a write and record when later writes are submitted. The test should verify that a second mutation cannot submit its write while the first is pending, then reload the final bytes and confirm they match the newest committed in-memory state.

[elnafateh]

Hi @joostjager, when you have a moment, would you mind taking a look at this PR?

It looks like @tnull is tied up with other priorities right now. I'd appreciate your feedback whenever you have the chance!

[joostjager]

At a high level, my view is that the root of this problem is that rust-lightning and ldk-node maintain separate event queues whose state can diverge. I think we should explore a way to reconcile the queues into one.