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 and channel state storage through PostgreSQL without a feature-gated server build. Keep local disk storage for server-owned files and history, and document the backup boundary.

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.

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

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

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-07-07T23:17:45Z

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

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.

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

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

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-07-07T23:17:45Z

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

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.

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

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

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-07-07T23:17:45Z

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

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.

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

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

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-07-07T23:17:45Z

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

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.

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

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

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-07-07T23:17:45Z

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

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.

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

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

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-07-07T23:17:45Z

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

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.

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

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

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.

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 a4378eaCompare July 12, 2026 19:41

benthecarman force-pushed the postgres branch from a4378ea to 261125aCompare 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 f25cb1dCompare 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 2b96b2bCompare August 3, 2026 07:17

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 a4378eaCompare July 12, 2026 19:41

benthecarman force-pushed the postgres branch from a4378ea to 261125aCompare 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 f25cb1dCompare 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 2b96b2bCompare August 3, 2026 07:17

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 a4378eaCompare July 12, 2026 19:41

benthecarman force-pushed the postgres branch from a4378ea to 261125aCompare 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 f25cb1dCompare 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 2b96b2bCompare August 3, 2026 07:17

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 a4378eaCompare July 12, 2026 19:41

benthecarman force-pushed the postgres branch from a4378ea to 261125aCompare 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 f25cb1dCompare 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 2b96b2bCompare August 3, 2026 07:17

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 a4378eaCompare July 12, 2026 19:41

benthecarman force-pushed the postgres branch from a4378ea to 261125aCompare 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 f25cb1dCompare 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 2b96b2bCompare August 3, 2026 07:17

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 a4378eaCompare July 12, 2026 19:41

benthecarman force-pushed the postgres branch from a4378ea to 261125aCompare 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 f25cb1dCompare 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 2b96b2bCompare August 3, 2026 07:17

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 a4378eaCompare July 12, 2026 19:41

benthecarman force-pushed the postgres branch from a4378ea to 261125aCompare 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 f25cb1dCompare 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 2b96b2bCompare August 3, 2026 07:17

Uh oh!

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Uh oh!

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!

benthecarman commented Jul 7, 2026•
edited
Loading

ldk-reviews-bot commented Jul 7, 2026•
edited
Loading

benthecarman commented Jul 7, 2026•
edited
Loading

ldk-reviews-bot commented Jul 7, 2026•
edited
Loading

benthecarman commented Jul 7, 2026•
edited
Loading

ldk-reviews-bot commented Jul 7, 2026•
edited
Loading