Expose Postgres storage backend by benthecarman · Pull Request #243 · lightningdevkit/ldk-server

benthecarman · 2026-07-07T23:17:45Z

Allow ldk-server to configure LDK Node wallet state, channel state, and payment history through PostgreSQL without a feature-gated server build. Keep local disk storage for server-owned files and forwarded-payment history, and document the backup and per-network isolation boundaries.

Refuse switching between SQLite and PostgreSQL after initialization. The local marker records only the backend type; it does not detect a changed PostgreSQL database or table, or missing state.

Limit PostgreSQL certificate reads to 1 MiB and test oversized and unbounded inputs. Remove the Getting Started claim that no other external dependencies are required.

Validation: formatting, default and all-feature tests, and Clippy passed.

AI-assisted-by: OpenAI Codex

ldk-reviews-bot · 2026-07-07T23:17:47Z

👋 Thanks for assigning @tankyleo 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.

tankyleo

skimmed briefly one question

tankyleo · 2026-07-28T02:52:40Z

+
+When `[storage.postgres]` is configured, LDK Node wallet/channel state is stored in
+PostgreSQL instead of `ldk_node_data.sqlite`. The storage directory remains required for
+the mnemonic, API key, TLS material, logs, and `ldk_server_data.sqlite`.


Would big routing nodes want ldk_server_data.sqlite to be in the postgres db too, given that they'll be forwarding a lot of payments ? Or is that for a later PR / out-of-scope here ?

It will be after lightningdevkit/ldk-node#772, just keeping the docs accurate of the local state of the repo. Will update once we merge and upgrade here

tankyleo · 2026-07-28T20:01:17Z

+
+# Optional LDK Node PostgreSQL storage for wallet and channel state.
+#[storage.postgres]
+#connection_string = "postgresql://postgres:postgres@localhost:5432" # PostgreSQL connection string. Do not include dbname if db_name is set.


If it works I find the key value format to be cleaner, less error prone ie host=localhost port=5432 user=postgres sslmode=disable

tbh i always hate these, but can switch if we really want

tankyleo · 2026-07-28T20:02:49Z

+```
+
+Only `connection_string` is required. `db_name`, `kv_table_name`, and `certificate_path`
+are optional. If `db_name` is set, do not also include a database name in the connection


Don't remember us explicitly rejecting a database name in the connection string anywhere in this patch ? Would it be good to add test coverage for this?

that's in ldk-node, added a test for this

tankyleo · 2026-07-28T20:04:00Z

+
+```toml
+[storage.postgres]
+connection_string = "postgresql://postgres:postgres@localhost:5432"


Similar here, the key-value format would be better if we can have it.

tankyleo · 2026-07-28T20:05:45Z

+			|| args.storage_postgres_kv_table_name.is_some()
+			|| args.storage_postgres_certificate_path.is_some()
+		{
+			let mut postgres = self.ldk_node_postgres.take().unwrap_or_default();


Here we can do a get_or_insert_default, so we don't need the assignment at the end of the block.

tankyleo · 2026-07-28T20:06:20Z

Similar here can do a get_or_insert_default if you care to :)

tankyleo · 2026-07-28T20:07:56Z

+			LdkNodeStorageConfig::Postgres {
+				connection_string: postgres
+					.connection_string
+					.ok_or_else(|| missing_field_err("storage_postgres_connection_string"))?,


For this field I wonder if this is a sensible default:
host=localhost port=5432 user=postgres sslmode=disable

Imo probably better to not have a defualt connection string

joostjager · 2026-07-31T06:43:35Z

Posting some AI comments that look sensible. Second one is in flight in lightningdevkit/ldk-node#1000, but maybe it should be a prerequisite.

🤖 Requesting changes for two fund-safety blockers.

The backend is selected only after reusing the existing mnemonic, with no migration or continuity check. Adding [storage.postgres] to an existing SQLite deployment therefore preserves the same Lightning identity while an empty PostgreSQL target creates fresh wallet and channel-manager state and silently ignores ldk_node_data.sqlite. For a node with live channels, starting without its channel monitors can lose channel funds. Please require an explicit verified migration, persist and validate backend identity, or at minimum refuse this switch when local SQLite LDK state exists and the PostgreSQL destination is empty.
The PostgreSQL build path does not acquire exclusive ownership of the database and table. Two deployments with the same mnemonic and target can both pass descriptor and network validation, read the same snapshot, and start the same node identity. The pinned PostgreSQL store uses process-local ordering locks and unconditional upserts, so overlapping instances can interleave stale channel-manager and monitor writes. Please hold a database advisory lock or equivalent lease for the node lifetime and fail the second writer before building the node.

The previous cross-network concern is fail-closed in the pinned dependency because the persisted BDK network is checked during wallet load. The target is still not network-isolated, however, so the operations claim that multiple networks can share one storage root without conflicts should state that PostgreSQL deployments need distinct database or table targets per network.

benthecarman · 2026-08-03T07:11:57Z

Made it so we forbid migrating. As this is 0.1, i think this is fine for now and we can make a proper implementation in the future.
Made Lock Postgres stores on initialization ldk-node#1012

joostjager · 2026-08-03T09:37:29Z

I’m not sure we should rush PostgreSQL support into ldk-server with an interim locking mechanism, only to replace it with a different approach later. That could also leave us with migration or compatibility concerns between deployed versions? lightningdevkit/ldk-node#1012 does not look entirely trivial either.

benthecarman · 2026-09-08T23:53:11Z

Now that #258 is merged with lightningdevkit/ldk-node#1012 this should be good to go

tankyleo

Here's a review pass from codex. Perhaps the first item is out of scope. Also told codex user starting two instances with the same network_dir is out of scope.

[P1] Reject missing or changed PostgreSQL state on restart _ /home/ubuntu/ldk-server/ldk-server/src/main.rs:791-792
When a node has already run on PostgreSQL and the configured database or table is later dropped, changed, or mistyped, postgres_lock_exists is true but this match accepts every PostgreSQL configuration as long as no SQLite file exists. build_with_postgres_store can then create an empty table and start fresh state under the same mnemonic and node ID, potentially endangering existing channels. Record and verify a backend identity or sentinel on subsequent PostgreSQL starts rather than only recording the backend type.
[P2] Bound the PostgreSQL certificate read _ /home/ubuntu/ldk-server/ldk-server/src/util/config.rs:558-561
When certificate_path points to an oversized file or an unbounded pseudo-file such as /dev/zero, fs::read_to_string reads until EOF before PEM parsing, so startup can exhaust memory or hang. Other externally configured files use read_to_string_with_limit; apply an appropriate certificate-size limit here as well.
[P2] Keep default builds independent of system OpenSSL _ /home/ubuntu/ldk-server/ldk-server/Cargo.toml:16-16
On Linux, enabling storage-postgres pulls in native-tls and openssl-sys without vendoring, so the default cargo build now fails unless pkg-config and OpenSSL development headers are installed, even when PostgreSQL is unused. This contradicts docs/getting-started.md stating that no other external dependencies are required; use the vendored TLS feature, make the backend opt-in, or document and install the new system prerequisites.
[P1] Filter PostgreSQL parameter values from debug logs _ /home/ubuntu/ldk-server/ldk-server/src/main.rs:1088-1088
When [storage.postgres] is used with the default Debug log level, tokio_postgres::query logs every bound parameter, including the complete Vec passed to each persistence write. Passing this store to LDK Node therefore copies serialized payment records_including preimages_and channel/wallet state into the console or system journal and the server log file, while large state blobs can also rapidly consume log storage. Filter the tokio_postgres query targets or otherwise prevent driver parameter logging before using this store.

benthecarman · 2026-09-15T22:21:58Z

Here's a review pass from codex. Perhaps the first item is out of scope. Also told codex user starting two instances with the same network_dir is out of scope.

* [P1] Reject missing or changed PostgreSQL state on restart _ /home/ubuntu/ldk-server/ldk-server/src/main.rs:791-792
  When a node has already run on PostgreSQL and the configured database or table is later dropped, changed, or mistyped, postgres_lock_exists is true but this match accepts every PostgreSQL configuration as long as no SQLite file exists. build_with_postgres_store can then create an empty table and start fresh state under the same mnemonic and node ID, potentially endangering existing channels. Record and verify a backend identity or sentinel on subsequent PostgreSQL starts rather than only recording the backend type.

yeah agreed this is out of scope, seems overkill

* [P2] Bound the PostgreSQL certificate read _ /home/ubuntu/ldk-server/ldk-server/src/util/config.rs:558-561
  When certificate_path points to an oversized file or an unbounded pseudo-file such as /dev/zero, fs::read_to_string reads until EOF before PEM parsing, so startup can exhaust memory or hang. Other externally configured files use read_to_string_with_limit; apply an appropriate certificate-size limit here as well.

fixed

* [P2] Keep default builds independent of system OpenSSL _ /home/ubuntu/ldk-server/ldk-server/Cargo.toml:16-16
  On Linux, enabling storage-postgres pulls in native-tls and openssl-sys without vendoring, so the default cargo build now fails unless pkg-config and OpenSSL development headers are installed, even when PostgreSQL is unused. This contradicts docs/getting-started.md stating that no other external dependencies are required; use the vendored TLS feature, make the backend opt-in, or document and install the new system prerequisites.

this is an explicit choice, we want to use system tls because we don't want to have to ship an update for every openssl security update, user can update themselves. However, did remove the line in docs saying we don't need external deps.

* [P1] Filter PostgreSQL parameter values from debug logs _ /home/ubuntu/ldk-server/ldk-server/src/main.rs:1088-1088
  When [storage.postgres] is used with the default Debug log level, tokio_postgres::query logs every bound parameter, including the complete Vec passed to each persistence write. Passing this store to LDK Node therefore copies serialized payment records_including preimages_and channel/wallet state into the console or system journal and the server log file, while large state blobs can also rapidly consume log storage. Filter the tokio_postgres query targets or otherwise prevent driver parameter logging before using this store.

discussed offline, seems overkill and weird to override user choice.

Allow ldk-server to configure LDK Node wallet and channel state storage through PostgreSQL without a feature-gated server build. Refuse switching between SQLite and PostgreSQL after initialization so the same node identity cannot silently start without its persisted channel state. Clarify the backup and per-network isolation boundaries. AI-assisted-by: OpenAI Codex

ldk-reviews-bot requested a review from valentinewallace July 7, 2026 23:28

benthecarman force-pushed the postgres branch 2 times, most recently from c79c1a4 to a4378ea Compare July 12, 2026 19:41

benthecarman force-pushed the postgres branch from a4378ea to 261125a Compare July 25, 2026 22:04

benthecarman requested review from tankyleo and removed request for valentinewallace July 28, 2026 00:44

tankyleo reviewed Jul 28, 2026

View reviewed changes

tankyleo self-requested a review July 28, 2026 02:58

tankyleo reviewed Jul 28, 2026

View reviewed changes

benthecarman force-pushed the postgres branch from 261125a to f25cb1d Compare July 30, 2026 04:59

benthecarman requested a review from tankyleo July 31, 2026 03:22

benthecarman force-pushed the postgres branch from f25cb1d to 2b96b2b Compare August 3, 2026 07:17

benthecarman force-pushed the postgres branch from 2b96b2b to 81eab49 Compare September 8, 2026 23:51

benthecarman force-pushed the postgres branch from 81eab49 to aa90ea3 Compare September 13, 2026 04:16

tankyleo reviewed Sep 15, 2026

View reviewed changes

benthecarman force-pushed the postgres branch from 3b7f12b to 0dea4e1 Compare September 15, 2026 22:16

benthecarman requested a review from tankyleo September 15, 2026 22:22

benthecarman force-pushed the postgres branch from 0dea4e1 to 9081bf8 Compare September 15, 2026 22:23

benthecarman force-pushed the postgres branch from 9081bf8 to 2c9b1a1 Compare September 17, 2026 21:57

Conversation

benthecarman commented Jul 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

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

Uh oh!

Uh oh!

tankyleo left a comment

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!

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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

joostjager commented Jul 31, 2026

Uh oh!

benthecarman commented Aug 3, 2026

Uh oh!

joostjager commented Aug 3, 2026

Uh oh!

benthecarman commented Sep 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tankyleo left a comment • 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!

benthecarman commented Sep 15, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

benthecarman commented Jul 7, 2026 •

edited

Loading

ldk-reviews-bot commented Jul 7, 2026 •

edited

Loading

benthecarman commented Sep 8, 2026 •

edited

Loading

tankyleo left a comment •

edited

Loading