EventQueue persistence failures can duplicate, skip, or resurrect events

[ben-kaufman]

TLDR

EventQueue::add_event and EventQueue::event_handled mutate the in-memory queue before asynchronously persisting the new snapshot.

This creates several failure modes:

• If add_event persistence fails, the event remains in memory. The LDK event handler returns ReplayEvent, so the source event can be replayed and added to the queue a second time.
• If event_handled persistence fails, the event has already been removed from memory. Retrying event_handled can remove the next event instead.
• Queue mutations and persistence are not serialized as one operation. Custom KVStore implementations can persist snapshots out of order. The built-in persistent stores mitigate ordinary completion reordering using per-key versions, but a smaller version-ordering race remains if execution is delayed between the queue mutation and the call to KVStore::write.

Vulnerable code

src/event.rs:409:

pub(crate) async fn add_event(&self, event: Event) -> Result<(), Error> {
	let data = {
		let mut locked_queue = self.queue.lock().expect("lock");
		locked_queue.push_back(event);
		EventQueueSerWrapper(&locked_queue).encode()
	};

	self.persist_queue(data).await?;

	if let Some(waker) = self.waker.lock().expect("lock").take() {
		waker.wake();
	}
	Ok(())
}

pub(crate) async fn event_handled(&self) -> Result<(), Error> {
	let data = {
		let mut locked_queue = self.queue.lock().expect("lock");
		locked_queue.pop_front();
		EventQueueSerWrapper(&locked_queue).encode()
	};

	self.persist_queue(data).await?;

	if let Some(waker) = self.waker.lock().expect("lock").take() {
		waker.wake();
	}
	Ok(())
}

Persistence occurs after the queue mutex has been released:

src/event.rs:448:

async fn persist_queue(&self, encoded_queue: Vec<u8>) -> Result<(), Error> {
	KVStore::write(
		&*self.kv_store,
		EVENT_QUEUE_PERSISTENCE_PRIMARY_NAMESPACE,
		EVENT_QUEUE_PERSISTENCE_SECONDARY_NAMESPACE,
		EVENT_QUEUE_PERSISTENCE_KEY,
		encoded_queue,
	)
	.await
	.map_err(|e| {
		log_error!(
			self.logger,
			"Write for key {}/{}/{} failed due to: {}",
			EVENT_QUEUE_PERSISTENCE_PRIMARY_NAMESPACE,
			EVENT_QUEUE_PERSISTENCE_SECONDARY_NAMESPACE,
			EVENT_QUEUE_PERSISTENCE_KEY,
			e
		);
		Error::PersistenceFailed
	})?;

	Ok(())
}

Duplicate events after a failed enqueue

When adding a user-facing event fails, the relevant LDK event handlers return ReplayEvent:

src/event.rs:1175:

let event = Event::PaymentReceived {
	payment_id: Some(payment_id),
	payment_hash,
	amount_msat,
	custom_records: onion_fields
		.map(|cf| cf.custom_tlvs().into_iter().map(|tlv| tlv.into()).collect())
		.unwrap_or_default(),
};

match self.event_queue.add_event(event).await {
	Ok(_) => return Ok(()),
	Err(e) => {
		log_error!(self.logger, "Failed to push to event queue: {}", e);
		return Err(ReplayEvent());
	},
};

ReplayEvent asks the LDK event provider to retry the event. However, the first add_event call has already pushed its event into the in-memory queue.

For PaymentReceived, replay reaches the enqueue again even when the payment-store update is unchanged:

src/event.rs:1152:

match self.payment_store.update(update).await {
	Ok(DataStoreUpdateResult::Updated)
	| Ok(DataStoreUpdateResult::Unchanged) => (
		// No need to do anything if the idempotent update was applied,
		// which might be the result of a replayed event.
	),
	// ...
}

If the retrying queue write succeeds, both copies can be persisted.

Incorrect acknowledgement behavior

Node::event_handled returns the persistence error to the caller:

src/lib.rs:981:

pub fn event_handled(&self) -> Result<(), Error> {
	let fut = self.event_queue.event_handled();
	self.runtime.block_on(fut).map_err(|e| {
		log_error!(
			self.logger,
			"Couldn't mark event handled due to persistence failure: {}",
			e
		);
		e
	})
}

But the failed operation has already removed the event from the in-memory queue. Retrying event_handled therefore removes the following event.

It also contradicts the documented behavior that the same event remains available until handling is successfully confirmed:

src/lib.rs:966:

/// Returns the next event in the event queue.
///
/// Will block the current thread until the next event is available.
///
/// **Note:** this will always return the same event until handling is confirmed
/// via [`Node::event_handled`].

Persistence ordering

Because the queue mutex is released before KVStore::write is called, a concurrent enqueue or acknowledgement can create and submit a newer snapshot while an older snapshot is still pending.

For custom KVStore implementations without same-key write ordering, the older write can complete last and overwrite the newer state.

The built-in SqliteStore, VssStore, and PostgresStore mitigate normal completion reordering by assigning versions when write is called, serializing same-key writes, and skipping stale versions.

However, mutation order and version-allocation order are not atomic. If operation A mutates the queue and is delayed before calling write, operation B can perform a later mutation but obtain the earlier store version. When A resumes, its older snapshot receives the newer version and can become the final persisted state.

This remaining window is small for the built-in stores but is not eliminated by their write-version handling.

Impact

A transient event-queue persistence failure can cause:

• Duplicate application-facing events after an LDK replay.
• The next event being silently acknowledged when an application retries event_handled.
• An acknowledged event being resurrected after restart.
• A newer event being absent after restart because an older snapshot became the final persisted value.

A duplicated PaymentReceived event could cause an application that treats events as exactly-once notifications to credit the same payment twice. Applications can mitigate this by deduplicating using payment_id and reconciling against PaymentStore, but the queue itself currently creates the duplicate.

This does not corrupt authoritative ChannelManager or ChannelMonitor state, which has separate persistence and replay handling. The impact is on the application-facing notification and accounting stream.

Issues #970, #1003, and #1020 concern related persistence or event-history behavior but do not appear to track this mutation-before-persistence issue.

Verification

The behavior is confirmed by source inspection of upstream main.

It can be reproduced with a fault-injecting KVStore:

1. Make the first add_event write fail.
2. Verify that the event nevertheless remains in the in-memory queue.
3. Replay the source LDK event.
4. Allow the second write to succeed.
5. Verify that the persisted queue contains two copies of the user-facing event.

The acknowledgement case can be reproduced by:

1. Enqueueing events A and B.
2. Making the write for event_handled fail.
3. Verifying that next_event now returns B despite the failed acknowledgement.
4. Calling event_handled again and verifying that B is also removed.

Ordering tests should also cover concurrent enqueue and acknowledgement operations with a controllable custom store.

Proposed fix

Serialize each queue operation and only commit the in-memory mutation after persistence succeeds:

let _operation_guard = self.operation_lock.lock().await;

let next_queue = {
	let current = self.queue.lock().expect("lock");
	let mut next = current.clone();

	// Apply the enqueue or acknowledgement to `next`.

	next
};

self.persist_queue(EventQueueSerWrapper(&next_queue).encode()).await?;
*self.queue.lock().expect("lock") = next_queue;

The operation lock must cover snapshot creation, persistence, and the in-memory commit. This ensures that:

• A failed enqueue leaves no in-memory copy for an LDK replay to duplicate.
• A failed acknowledgement leaves the original event at the front.
• Persistence writes are submitted in the same order as queue mutations.
• Only successfully persisted state becomes visible in memory.

Regression tests should cover failed enqueue replay, failed acknowledgement retries, concurrent enqueue/acknowledgement operations, custom-store write reordering, and restart behavior.