Add PaginatedKVStore traits upstreamed from ldk-server #4347

benthecarman · 2026-01-26T15:00:16Z

Allows for a paginated KV store for more efficient listing of keys so you don't need to fetch all at once. Uses monotonic counter or timestamp to track the order of keys and allow for pagination. The traits are largely just copy-pasted from ldk-server.

This also adds a PaginatedKVStoreSyncAdapter/PaginatedKVStoreAdapter so you can use a paginated kv store in contexts that expect a regular key value store.

Adds some basic tests that were generated using claude code.

ldk-reviews-bot · 2026-01-26T15:00:20Z

👋 I see @tnull was un-assigned.
If you'd like another reviewer assignment, please click here.

codecov · 2026-01-26T16:41:29Z

Codecov Report

❌ Patch coverage is 65.49020% with 88 lines in your changes missing coverage. Please review.
✅ Project coverage is 86.03%. Comparing base (3fee76b) to head (3a60588).
⚠️ Report is 20 commits behind head on main.

Files with missing lines	Patch %	Lines
lightning-persister/src/fs_store.rs	65.74%	30 Missing and 7 partials ⚠️
lightning/src/util/persist.rs	56.00%	33 Missing ⚠️
lightning/src/util/test_utils.rs	75.00%	17 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4347      +/-   ##
==========================================
- Coverage   86.08%   86.03%   -0.06%     
==========================================
  Files         156      156              
  Lines      102416   102717     +301     
  Branches   102416   102717     +301     
==========================================
+ Hits        88165    88370     +205     
- Misses      11759    11843      +84     
- Partials     2492     2504      +12

Flag	Coverage Δ
tests	`86.03% <65.49%> (-0.06%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

TheBlueMatt · 2026-01-26T16:45:11Z

lightning/src/util/persist.rs

+
+	/// A token that can be used to retrieve the next set of keys.
+	/// The `String` is the last key from the current page and the `i64` is
+	/// the associated `time` used for ordering.


I don't think most DBs are going to support this? "sort by the last-changed as of time X" isn't possible without storing what the last-updated time was prior to X when something is updated after X.

ldk-server does it based on creation_at not updated_at, will update the docs to make that more clear

Hmm, do all of our backend storage things have a created-at time? I guess if you want it time-ordered you have to do it that way but it adds quite a requirement :/

for ldk-server's we do

https://github.com/lightningdevkit/ldk-server/blob/9ae5ff8e2d2faf19540ce1be7af973ea134f9465/ldk-server/src/io/persist/sqlite_store/mod.rs#L97

Hmm, presumably that needs to be upstreamed to ldk-node and maybe also vss-server? I imagine we want pagination there too?

lightning/src/util/persist.rs

TheBlueMatt · 2026-01-26T18:09:37Z

lightning/src/util/persist.rs

+/// when dealing with a large number of keys that cannot be efficiently retrieved all at once.
+///
+/// For an asynchronous version of this trait, see [`PaginatedKVStore`].
+pub trait PaginatedKVStoreSync: Send + Sync {


Why doesn't this just extend KVStoreSync? Seems weird to duplicate the interface.

I am too used to having to create wrappers for ldk traits, I am actually in ldk now though! Fixed

You can add extension traits in downstream crates too?

You can add extension traits in downstream crates too?

To give some 'historic' context here, there were discussions on whether or not to make this an extension trait (cf. lightningdevkit/ldk-server#22). In the end, the PR author decided against it, also because it was anticipated that pagination might require two separate implementations.

However, IMO, now would be a good time to at least consider rethinking and double-checking some the design choices made, as there is still time to fundamentally LDK Server's persistence model, before we cut the first release (cc @jkczyz as he was also part of the original discussions, not sure if he has an opinion now).

In particular, I want to throw out some questions to consider:

Do we really want to maintain separate implementations for all backends and have LDK Node take different store types for 'general purpose' vs 'payment data'?

Either way, couldn't we still make PaginatedKVStore an extension trait?

If we will require it in LDK Node for payment data, will we be able to add pagination to all supported backends? if so, should this even be just a breaking change to KVStore::list rather than making it an extension trait?

Somewhat relatedly, note that from my point of view our endgoal should be that also all backend implementations (incl. VSS, SQLite, Postgres) eventually live in lightning-persister, and that preferably very little or no custom logic would live on the LDK Node/Server side. IMO this top-down approach would give clear interfaces and implementations across the board and avoid risking running into edge cases when making conflicting decisions on implementation details in different crates.

Do we really want to maintain separate implementations for all backends and have LDK Node take different store types for 'general purpose' vs 'payment data'?

Ideally not indeed.

Either way, couldn't we still make PaginatedKVStore an extension trait?

Definitely should be (it is now in this PR).

If we will require it in LDK Node for payment data, will we be able to add pagination to all supported backends?

Yea, see above discussion. Requiring that the paginated list be in created_at order means that most of our backends will have to actually store a created_at time (or have an incrementing row id at least). At least the only one in lightning-persister (filesystem store) already has it, but the SQLite/Postgres/VSS backends will all need it.

For that reason I'm really not a fan of sorting by created_at but it does certainly make the client easier. Another option would be to only require that things be listed in key-order and then make sure that when we want pagination we store a key that is ordered, but that doesn't work for VSS cause the key is encrypted garbage (so we'd have to have some crazy abstraction-break there). Dunno if anyone has any more creative ideas.

Somewhat relatedly, note that from my point of view our endgoal should be that also all backend implementations (incl. VSS, SQLite, Postgres) eventually live in lightning-persister, and that preferably very little or no custom logic would live on the LDK Node/Server side. IMO this top-down approach would give clear interfaces and implementations across the board and avoid risking running into edge cases when making conflicting decisions on implementation details in different crates.

👍

benthecarman · 2026-01-28T15:58:55Z

Updated with @TheBlueMatt's suggestions, now only uses last_key as the page token and leaves the time/ordering up to the implementation

benthecarman · 2026-01-28T19:07:34Z

implemented the paginated kv store for the file system store

tnull · 2026-01-29T12:07:45Z

lightning/src/util/persist.rs

+	/// Returns a paginated list of keys that are stored under the given `secondary_namespace` in
+	/// `primary_namespace`, ordered from most recently created to least recently created.
+	///
+	/// Implementations must return keys in reverse creation order (newest first). How creation


Hmm, I think this might have been discussed above, but could you reiterate why we do creation time, not last update time? The latter seems like the obvious contender to me when we'd like to list payments, in particular, so it seems this might require that we go through all entries and sort them in-memory again for the most common usecase?

the problem with using updated at time is that it can be prone to race conditions, added to the docs

tnull · 2026-01-29T12:13:19Z

lightning/src/util/persist.rs

+	/// A vector of keys, ordered from most recently created to least recently created.
+	pub keys: Vec<String>,
+
+	/// The last key in this page, which can be passed to the next call to continue pagination.


While using the last key specifically matches what we do in VSS, requiring the page token to be the last key is a stricter requirement than what the VSS interface guarantees. Could we maybe just make this an opaque next_page_token field without any special semantics? Then the implementation could still use the last key if it works for it, but we wouldn't require specific semantics?

okay switched to something more generic

Allows for a paginated KV store for more efficient listing of keys so you don't need to fetch all at once. Uses monotonic counter or timestamp to track the order of keys and allow for pagination. The traits are largely just copy-pasted from ldk-server. Adds some basic tests that were generated using claude code.

Test created with claude code

TheBlueMatt · 2026-01-29T14:49:56Z

lightning-persister/src/fs_store.rs

+						// Get file creation time, falling back to modified time if unavailable.
+						let metadata = entry.metadata()?;
+						let created_time = metadata
+							.created()


I believe in most filesystems the created time is actually gonna be the last-update time because we overwrite-on-update. Thus, I don't think its possible to implement the pagination for the filesystem store (without including additional metadata in the filenames).

Sorry I had earlier assumed this would work but I dont think so.

TheBlueMatt · 2026-01-29T14:50:44Z

lightning-persister/src/fs_store.rs

+		assert_eq!(response.keys[0], "key_e");
+		assert_eq!(response.keys[4], "key_a");


You can assert is_sorted

benthecarman requested a review from tnull January 26, 2026 15:00

TheBlueMatt reviewed Jan 26, 2026

View reviewed changes

benthecarman force-pushed the paginated-kv-store branch from 5160a11 to 3fb869f Compare January 26, 2026 17:56

TheBlueMatt reviewed Jan 26, 2026

View reviewed changes

benthecarman force-pushed the paginated-kv-store branch 2 times, most recently from ebb2e5d to 95b82f8 Compare January 26, 2026 20:45

tnull removed their request for review January 27, 2026 08:30

benthecarman force-pushed the paginated-kv-store branch 2 times, most recently from ffdf5c9 to 77d2cac Compare January 28, 2026 15:57

benthecarman force-pushed the paginated-kv-store branch 2 times, most recently from 0105148 to ac4e907 Compare January 28, 2026 19:07

tnull reviewed Jan 29, 2026

View reviewed changes

tnull mentioned this pull request Jan 29, 2026

Add storage for forwarded payments lightningdevkit/ldk-node#772

Open

benthecarman added 2 commits January 29, 2026 08:44

Implement PaginatedKVStore for FilesystemStore

3a60588

Test created with claude code

benthecarman force-pushed the paginated-kv-store branch from ac4e907 to 3a60588 Compare January 29, 2026 14:45

benthecarman self-assigned this Jan 29, 2026

TheBlueMatt reviewed Jan 29, 2026

View reviewed changes

benthecarman added this to Weekly Goals Jan 29, 2026

		assert_eq!(response.keys[0], "key_e");
		assert_eq!(response.keys[4], "key_a");

Add PaginatedKVStore traits upstreamed from ldk-server #4347

Are you sure you want to change the base?

Add PaginatedKVStore traits upstreamed from ldk-server #4347

Uh oh!

Conversation

benthecarman commented Jan 26, 2026

Uh oh!

ldk-reviews-bot commented Jan 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented Jan 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tnull Jan 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

benthecarman commented Jan 28, 2026

Uh oh!

benthecarman commented Jan 28, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

ldk-reviews-bot commented Jan 26, 2026 •

edited

Loading

codecov bot commented Jan 26, 2026 •

edited

Loading

tnull Jan 27, 2026 •

edited

Loading