Clarify that all removals must be well-ordered, even `lazy` ones
What the developer wrote
Authored by Matt Corallo
Clarify that all removals must be well-ordered, even `lazy` ones
In the previous commit we reverted
561da4cfb8cd27085b124ae1af96a8745f7f31dc. One of the motivations
for it (in addition to `lazy` removals being somewhat less, though
still arguably useful in an async context) was that the ordering
requirements of `lazy` removals is somewhat unclear.
Here we simply default to the simplest safe option, requiring a
total order across all `write` and `remove` operations to the same
key, `lazy` or not.
What changed, and why it matters
This commit only changes documentation comments in a Rust source file. It clarifies that storage removals (including lazy ones) must stay in a consistent order with writes to the same key. There is no code change, so it does not by itself fix a runnable bug or introduce a vulnerability. It is a safety clarification for developers building storage backends.
No immediate action is required for end users. Storage backend implementers should review the clarified contract and ensure their `KVStore` implementation orders writes and removes to the same key consistently, with later writes canceling pending lazy removals. Consider auditing existing implementations for compliance with the clarified contract.
Security signals we found
Documentation-only change
Clarifies ordering requirement for lazy removals
Mentions crash/replay safety of lazy removals
Refers to a prior revert of commit 561da4cfb8cd27085b124ae1af96a8745f7f31dc
Evidence from the diff
The diff adds four lines of doc comment to the KVStore::remove trait method in lightning/src/util/persist.rs. The new text requires that all remove operations complete in a consistent total order with write operations to the same key, and that a later write to a key with a pending removal must cancel/overwrite that removal. No implementation logic is modified. The commit message says this is the ‘simplest safe option’ after reverting a previous change that made lazy removal ordering unclear.
Changed components
lightning/src/util/persist.rsKVStore trait documentationInspect captured patch +5 / −0
diff --git a/lightning/src/util/persist.rs b/lightning/src/util/persist.rs
index 152986b..5d34603 100644
--- a/lightning/src/util/persist.rs
+++ b/lightning/src/util/persist.rs
@@ -255,6 +255,11 @@ pub trait KVStore {
/// potentially get lost on crash after the method returns. Therefore, this flag should only be
/// set for `remove` operations that can be safely replayed at a later time.
///
+ /// All removal operations must complete in a consistent total order with [`Self::write`]s
+ /// to the same key. Whether a removal operation is `lazy` or not, [`Self::write`] operations
+ /// to the same key which occur before a removal completes must cancel/overwrite the pending
+ /// removal.
+ ///
/// Returns successfully if no data will be stored for the given `primary_namespace`,
/// `secondary_namespace`, and `key`, independently of whether it was present before its
/// invokation or not.
Why this scored 20/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.