Postgres follow-ups

.github/workflows/e2e-tests.yml

@@ -13,6 +13,21 @@
   e2e-tests:
     runs-on: ubuntu-latest
 
+    services:
+      postgres:
+        image: postgres:latest
+        env:
+          POSTGRES_USER: postgres
+          POSTGRES_PASSWORD: postgres
+          POSTGRES_DB: ldk_server_e2e
+        ports:
+          - 5432/tcp
+        options: >-
+          --health-cmd "pg_isready -U postgres -d ldk_server_e2e"
+          --health-interval 5s
+          --health-timeout 5s
+          --health-retries 10
+
     steps:
       - name: Checkout code
         uses: actions/checkout@v6
@@ -39,3 +54,10 @@
         env:
           RUST_BACKTRACE: 1
           BITCOIND_SKIP_DOWNLOAD: 1
+
+      - name: Run PostgreSQL end-to-end tests
+        run: cargo test --manifest-path e2e-tests/Cargo.toml --test postgres --verbose --color=always -- --ignored --nocapture
+        env:
+          POSTGRES_CONNECTION_STRING: postgresql://postgres:postgres@127.0.0.1:${{ job.services.postgres.ports[5432] }}/ldk_server_e2e?sslmode=disable
+          RUST_BACKTRACE: 1
+          BITCOIND_SKIP_DOWNLOAD: 1

contrib/ldk-server-config.toml

@@ -41,7 +41,9 @@ dir_path = "/tmp/ldk-server/"                 # Path for local ldk-server data,
 #connection_string = "postgresql://postgres:postgres@localhost:5432" # PostgreSQL connection string. Do not include dbname if db_name is set.
 #db_name = "ldk_db"                         # Optional database name.
 #kv_table_name = "ldk_data"                 # Optional KV table name.
-#certificate_path = "/path/to/postgres-ca.pem" # Optional CA certificate PEM file for TLS PostgreSQL connections.
+#certificate_path = "/path/to/postgres-ca.pem" # PEM-encoded CA certificate required to enable PostgreSQL TLS.
+# Without certificate_path, the default sslmode=prefer uses plaintext; sslmode=require fails to connect.
+# With certificate_path, TLS is required; sslmode=disable is rejected.
 
 [log]
 level = "Debug"                               # Log level (Error, Warn, Info, Debug, Trace)

docs/configuration.md

@@ -131,10 +131,16 @@ kv_table_name = "ldk_data"
 certificate_path = "/path/to/postgres-ca.pem"
 ```
 
-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
-string. If `certificate_path` is set, the file must contain a PEM-encoded CA certificate
-for TLS PostgreSQL connections.
+Only `connection_string` is required. `db_name` and `kv_table_name` are optional. If
+`db_name` is set, do not also include a database name in the connection string.
+
+`certificate_path` is optional, but **required to enable PostgreSQL TLS**. The file must
+contain a PEM-encoded CA certificate, which is added to the system's default trusted roots.
+Without `certificate_path`, the default `sslmode=prefer` uses plaintext even when PostgreSQL
+supports TLS, and `sslmode=require` fails to connect. System trust alone does not enable TLS.
+
+When `certificate_path` is set, TLS is required, including with the default `sslmode=prefer`.
+Combining it with `sslmode=disable` is rejected.
 
 Storage migration is not supported. ldk-server refuses to start with PostgreSQL when an existing
 `ldk_node_data.sqlite` file is present. After the first successful PostgreSQL node build,

e2e-tests/src/lib.rs

@@ -139,8 +139,12 @@ pub enum ChainSource {
 		rpc_password: String,
 		rest_address: Option<String>,
 	},
-	Electrum { server_url: String },
-	Esplora { server_url: String },
+	Electrum {
+		server_url: String,
+	},
+	Esplora {
+		server_url: String,
+	},
 }
 
 impl ChainSource {
@@ -177,6 +181,7 @@ pub struct TestConfigBuilder {
 	grpc_service_address: String,
 	alias: Option<String>,
 	storage_dir: PathBuf,
+	postgres: Option<(String, String)>,
 	chain_source: ChainSource,
 	metrics_auth: Option<(String, String)>,
 	log: Option<(Option<String>, String)>,
@@ -194,6 +199,7 @@ impl TestConfigBuilder {
 			grpc_service_address: format!("127.0.0.1:{}", params.grpc_port),
 			alias: Some("e2e-test-node".to_string()),
 			storage_dir: params.storage_dir.clone(),
+			postgres: None,
 			chain_source: ChainSource::Bitcoind {
 				rpc_address: params.rpc_address.clone(),
 				rpc_user: params.rpc_user.clone(),
@@ -212,6 +218,12 @@ impl TestConfigBuilder {
 		self
 	}
 
+	/// Store LDK Node state in PostgreSQL, keeping keys and server files on disk.
+	pub fn postgres(mut self, connection_string: &str, kv_table_name: &str) -> Self {
+		self.postgres = Some((connection_string.to_string(), kv_table_name.to_string()));
+		self
+	}
+
 	/// Set the node alias, or `None` to omit it entirely.
 	pub fn alias(mut self, alias: Option<&str>) -> Self {
 		self.alias = alias.map(str::to_string);
@@ -319,6 +331,13 @@ poll_metrics_interval = 1{metrics_auth}
 			metrics_auth = metrics_auth,
 		);
 
+		if let Some((connection_string, kv_table_name)) = &self.postgres {
+			config.push_str(&format!(
+				"\n[storage.postgres]\nconnection_string = \"{}\"\nkv_table_name = \"{}\"\n",
+				connection_string, kv_table_name,
+			));
+		}
+
 		if let Some((level, file)) = &self.log {
 			config.push_str("\n[log]\n");
 			if let Some(level) = level {
@@ -357,27 +376,9 @@ impl LdkServerHandle {
 		config_bitcoind: &TestBitcoind, config: impl FnOnce(&TestServerParams) -> String,
 	) -> Self {
 		let (mut child, params, config_path) = spawn_server(config_bitcoind, config);
+		forward_server_output(&mut child);
 		let TestServerParams { grpc_port, p2p_port, storage_dir, .. } = params;
 
-		// Spawn threads to forward stdout and stderr for debugging
-		let stdout = child.stdout.take().unwrap();
-		std::thread::spawn(move || {
-			let reader = BufReader::new(stdout);
-			for line in reader.lines().map_while(Result::ok) {
-				eprintln!("[ldk-server stdout] {}", line);
-			}
-		});
-		let stderr = child.stderr.take().unwrap();
-		std::thread::spawn(move || {
-			let reader = BufReader::new(stderr);
-			for line in reader.lines().map_while(Result::ok) {
-				if line.contains("Failed to retrieve fee rate estimates") {
-					continue;
-				}
-				eprintln!("[ldk-server stderr] {}", line);
-			}
-		});
-
 		// Wait for the api_key and tls.crt files to appear in the network subdir
 		let network_dir = storage_dir.join("regtest");
 		let api_key_path = network_dir.join("api_key");
@@ -415,6 +416,18 @@ impl LdkServerHandle {
 		handle
 	}
 
+	/// Kill and restart the server with the same config and storage to test crash recovery.
+	pub async fn restart(&mut self) {
+		let mut child = self.child.take().expect("Server is not running");
+		child.kill().expect("Failed to kill ldk-server");
+		child.wait().expect("Failed to reap ldk-server");
+		let mut child = spawn_server_process(&self.config_path);
+		forward_server_output(&mut child);
+		self.child = Some(child);
+		let info = wait_for_server_ready(self, Duration::from_secs(60)).await;
+		assert_eq!(info.node_id, self.node_id, "Node identity changed after restart");
+	}
+
 	pub fn client(&self) -> &LdkServerClient {
 		&self.client
 	}
@@ -457,17 +470,42 @@ fn spawn_server(
 	let config_path = params.storage_dir.join("config.toml");
 	std::fs::write(&config_path, &config_content).unwrap();
 
+	let child = spawn_server_process(&config_path);
+	(child, params, config_path)
+}
+
+/// Spawn a server using an existing config, retaining its output pipes.
+fn spawn_server_process(config_path: &Path) -> Child {
 	let server_binary = server_binary_path();
-	let child = Command::new(&server_binary)
-		.arg(config_path.to_str().unwrap())
+	Command::new(&server_binary)
+		.arg(config_path)
 		.stdout(Stdio::piped())
 		.stderr(Stdio::piped())
 		.spawn()
 		.unwrap_or_else(|e| {
 			panic!("Failed to start ldk-server binary at {:?}: {}", server_binary, e)
-		});
+		})
+}
 
-	(child, params, config_path)
+fn forward_server_output(child: &mut Child) {
+	// Spawn threads to forward stdout and stderr for debugging
+	let stdout = child.stdout.take().unwrap();
+	std::thread::spawn(move || {
+		let reader = BufReader::new(stdout);
+		for line in reader.lines().map_while(Result::ok) {
+			eprintln!("[ldk-server stdout] {}", line);
+		}
+	});
+	let stderr = child.stderr.take().unwrap();
+	std::thread::spawn(move || {
+		let reader = BufReader::new(stderr);
+		for line in reader.lines().map_while(Result::ok) {
+			if line.contains("Failed to retrieve fee rate estimates") {
+				continue;
+			}
+			eprintln!("[ldk-server stderr] {}", line);
+		}
+	});
 }
 
 /// Start ldk-server with the given config and expect it to fail (exit non-zero).
@@ -793,9 +831,7 @@ pub async fn setup_funded_channel(
 		.open_channel(OpenChannelRequest {
 			node_pubkey: server_b.node_id().to_string(),
 			address: format!("127.0.0.1:{}", server_b.p2p_port),
-			amount: Some(open_channel_request::Amount::ChannelAmountSats(
-				channel_amount_sats,
-			)),
+			amount: Some(open_channel_request::Amount::ChannelAmountSats(channel_amount_sats)),
 			push_to_counterparty_msat: None,
 			channel_config: None,
 			announce_channel: true,

e2e-tests/tests/e2e.rs

@@ -1918,18 +1918,22 @@ async fn test_metrics_endpoint() {
 	assert!(metrics.contains("ldk_server_total_anchor_channels_reserve_sats 0"));
 	assert!(metrics.contains("ldk_server_total_lightning_balance_sats 0"));
 
-	// Set up channel and make a payment to trigger metrics update
+	// Set up the channel and confirm the wallet deposit and channel funding transaction.
 	setup_funded_channel(&bitcoind, &server_a, &server_b, 100_000).await;
+	mine_and_sync(&bitcoind, &[&server_a, &server_b], 6).await;
 
-	// Poll for channel, peer and balance metrics.
+	// Wait for both onchain payments and the channel, peer and balance metrics.
 	let timeout = Duration::from_secs(10);
 	let start = std::time::Instant::now();
 	loop {
 		let metrics = client.get_metrics().await.unwrap();
 		if metrics.contains("ldk_server_total_peers_count 1")
 			&& metrics.contains("ldk_server_total_channels_count 1")
 			&& metrics.contains("ldk_server_total_public_channels_count 1")
-			&& metrics.contains("ldk_server_total_payments_count 2")
+			&& metrics.contains("ldk_server_total_payments_count 2\n")
+			&& metrics.contains("ldk_server_total_successful_payments_count 2\n")
+			&& metrics.contains("ldk_server_total_pending_payments_count 0\n")
+			&& metrics.contains("ldk_server_total_failed_payments_count 0\n")
 			&& !metrics.contains("ldk_server_total_lightning_balance_sats 0")
 			&& !metrics.contains("ldk_server_total_onchain_balance_sats 0")
 			&& !metrics.contains("ldk_server_spendable_onchain_balance_sats 0")
@@ -1962,12 +1966,15 @@ async fn test_metrics_endpoint() {
 
 	run_cli(&server_a, &["bolt11-send", &invoice_resp.invoice]);
 
-	// Wait to receive the PaymentSuccessful event and update metrics
+	// The deposit, channel funding and BOLT11 payment must all be counted as successful.
 	let timeout = Duration::from_secs(30);
 	let start = std::time::Instant::now();
 	loop {
 		let metrics = client.get_metrics().await.unwrap();
-		if metrics.contains("ldk_server_total_successful_payments_count 1")
+		if metrics.contains("ldk_server_total_payments_count 3\n")
+			&& metrics.contains("ldk_server_total_successful_payments_count 3\n")
+			&& metrics.contains("ldk_server_total_pending_payments_count 0\n")
+			&& metrics.contains("ldk_server_total_failed_payments_count 0\n")
 			&& !metrics.contains("ldk_server_total_lightning_balance_sats 0")
 			&& !metrics.contains("ldk_server_total_onchain_balance_sats 0")
 			&& !metrics.contains("ldk_server_spendable_onchain_balance_sats 0")
@@ -1976,7 +1983,7 @@ async fn test_metrics_endpoint() {
 			break;
 		}
 		if start.elapsed() > timeout {
-			panic!("Timed out waiting for payment metrics to update");
+			panic!("Timed out waiting for payment metrics to update. Current metrics:\n{metrics}");
 		}
 		tokio::time::sleep(Duration::from_millis(500)).await;
 	}