Add Postgres database by benthecarman · Pull Request #863 · lightningdevkit/ldk-node

benthecarman · 2026-04-01T19:55:17Z

Add a PostgresStore implementation behind the postgres feature flag, mirroring the existing SqliteStore. Uses tokio-postgres (async-native) with an internal tokio runtime for the sync KVStoreSync trait, following the VssStore pattern.

Also gates sqlite behind a feature flag, since most tests relied on this, we ended up needing to gate a lot of them behind the flag. Curious on thoughts on how to handle this best

ldk-reviews-bot · 2026-04-01T19:55:21Z

👋 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.

tnull

Took a first high-level look.

Also gates sqlite behind a feature flag, since most tests relied on this, we ended up needing to gate a lot of them behind the flag. Curious on thoughts on how to handle this best

Not sure I'm following? Why do we want to feature-gate tests based on SQLite?

Also tagging @tankyleo as a secondary reviewer here as he has a lot of recent experience with Postgres on VSS Server.

.github/workflows/postgres-integration.yml

tnull · 2026-04-02T10:54:07Z

src/io/postgres_store/mod.rs

+
+	// An internal runtime we use to avoid any deadlocks we could hit when waiting on async
+	// operations to finish from a sync context.
+	internal_runtime: Option<tokio::runtime::Runtime>,


I think we could avoid using an internal runtime for this and rather just reuse crate::runtime::Runtime, if we wanted. Though, when upstreaming to LDK we'll need to rethink all of this anyways (cf. VssStore upstreaming PR).

Yeah I just copied VSS to be safe.

src/io/postgres_store/mod.rs

tnull · 2026-04-02T11:03:07Z

src/io/postgres_store/mod.rs

+		let kv_table_name = kv_table_name.unwrap_or(DEFAULT_KV_TABLE_NAME.to_string());
+
+		let (client, connection) =
+			tokio_postgres::connect(connection_string, NoTls).await.map_err(|e| {


We probably should support TLS from the getgo?

Also, we'll want users to pick non-default databases and create them if they don't exist yet. Note for this to work you'll first need to create a connection with the default, to then create the database you'll have in mind (see lightningdevkit/vss-server#55 and follow-ups for reference).

src/io/postgres_store/mod.rs

tnull · 2026-04-02T11:05:27Z

src/io/postgres_store/mod.rs

+	) -> io::Result<()> {
+		check_namespace_key_validity(primary_namespace, secondary_namespace, Some(key), "write")?;
+
+		self.execute_locked_write(inner_lock_ref, locking_key, version, async move || {


Super annoying that we have that replicated three times (at least) by now. We should look to DRY this up, when upstreaming to lightning-persister at the very latest.

Yeah I had same thought.

tnull · 2026-04-02T11:07:42Z

src/io/postgres_store/mod.rs

+	) -> io::Result<Vec<u8>> {
+		check_namespace_key_validity(primary_namespace, secondary_namespace, Some(key), "read")?;
+
+		let locked_client = self.client.lock().await;


If all operations take a MutexGuard and await the operation anyways, do we really need the write-lock-versioning? Isn't this practically sync?

Technically we'd still need this if someone is gonna do a join! on two writes to the same key. But yeah, could probably be okay to remove?

tnull · 2026-04-02T11:08:45Z

src/builder.rs


 	/// Builds a [`Node`] instance with a [`SqliteStore`] backend and according to the options
 	/// previously configured.
+	#[cfg(feature = "sqlite")]


Why do we suddenly introduce features? We so far held off and meant to do that in a dedicated PR at some point for this or next release. Is it necessary to make Postgres work?

okay removed for now

Add a PostgresStore implementation behind the "postgres" feature flag, mirroring the existing SqliteStore. Uses tokio-postgres (async-native) with an internal tokio runtime for the sync KVStoreSync trait, following the VssStore pattern. Includes unit tests, integration tests (channel full cycle and node restart), and a CI workflow that runs both against a PostgreSQL service container. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

- Add #[cfg(test)] to RefUnwindSafe impl (matches VssStore) - Take connection_string as String instead of &str - Bump actions/checkout to v6 in postgres CI workflow Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Add an optional db_name parameter (defaulting to ldk_node) that is always used to auto-create the database if it doesn't exist. When neither db_name nor a dbname in the connection string is provided the default is used. If both are set, return an error. Follows the pattern used by vss-server. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Check connection health before each operation using is_closed() and check_connection(). If the connection is dead, transparently create a new one. This prevents permanent failures when the PostgreSQL server restarts or the connection is otherwise dropped. Pattern modeled after vss-server's ensure_connected approach. Generated with AI (Claude Code) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Use native-tls and postgres-native-tls to support encrypted connections, following the same pattern as the VSS server. A new PostgresTlsConfig struct controls TLS behavior: - None: plaintext (existing behavior) - Some with certificate_pem: None: TLS using system root CAs - Some with certificate_pem: Some(pem): TLS with a custom CA Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Drop test tables at the start and end of each integration test to prevent stale data from prior runs causing WalletSetupFailed errors when seeds change between runs. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

The PostgreSQL integration tests need BITCOIND_EXE and ELECTRS_EXE environment variables to spawn regtest nodes, but the workflow was missing the download, caching, and env var steps that rust.yml has. Generated with the assistance of AI (Claude Code). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

benthecarman · 2026-04-03T20:00:49Z

Did fixes in follow up commits for ease of review.

ldk-reviews-bot · 2026-04-04T11:11:09Z

🔔 1st Reminder

Hey @tankyleo! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

benthecarman requested a review from tnull April 1, 2026 19:55

benthecarman force-pushed the postgres branch from c334535 to 8becefe Compare April 1, 2026 20:12

tnull reviewed Apr 2, 2026

View reviewed changes

tnull requested a review from tankyleo April 2, 2026 11:10

benthecarman added this to Weekly Goals Apr 2, 2026

benthecarman self-assigned this Apr 2, 2026

benthecarman and others added 2 commits April 2, 2026 18:30

Address PR review feedback

0090f1a

- Add #[cfg(test)] to RefUnwindSafe impl (matches VssStore) - Take connection_string as String instead of &str - Bump actions/checkout to v6 in postgres CI workflow Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

benthecarman force-pushed the postgres branch from 8becefe to b5d297e Compare April 3, 2026 19:34

benthecarman and others added 4 commits April 3, 2026 14:44

Clean up tables in PostgreSQL integration tests

e8f478b

Drop test tables at the start and end of each integration test to prevent stale data from prior runs causing WalletSetupFailed errors when seeds change between runs. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

benthecarman force-pushed the postgres branch from b5d297e to e8f478b Compare April 3, 2026 19:57

Conversation

benthecarman commented Apr 1, 2026

Uh oh!

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

Uh oh!

Uh oh!

tnull left a comment

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!

Uh oh!

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!

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!

benthecarman commented Apr 3, 2026

Uh oh!

ldk-reviews-bot commented Apr 4, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

ldk-reviews-bot commented Apr 1, 2026 •

edited

Loading