Bound incoming request and add postgres service by enigbe · Pull Request #76 · lightningdevkit/vss-server

enigbe · 2025-12-14T22:32:58Z

What this PR does

Adds a PostgreSQL Docker service for the Rust server, removing the need for a local PostgreSQL installation
Introduces a configurable maximum size limit for incoming request bodies, addressing a previously noted TODO
- Operators can optionally set maximum_request_body_size in [server_config] or via environment variable
- Defaults to 1 GB if not specified

ldk-reviews-bot · 2025-12-14T22:33:01Z

👋 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 · 2025-12-15T09:16:08Z

rust/server/src/vss_service.rs

 use std::pin::Pin;
 use std::sync::Arc;

+const MAXIMUM_REQUEST_BODY_SIZE: u16 = 65_535;


This seems very conservative, given that monitors could get quite large. Given that the VSS service is actually a storage service, it also might make sense to make this configurable (in contrast to lightningdevkit/ldk-server#80, but even there we set the limit to 10MB).

As mentioned elsewhere: I guess a static upper bound is a good first step, but if we're really concerned about DoS we might need some dynamic rate limiting on a per-IP basis. Although then the question becomes how much of that should be considered the concern of the VSS service itself, and how much we'd just expect users to slap a load balancer/Cloudflare in front of the service to handle that for them

I've made the configuration changes suggested here, capping at 20 MB for the maximum size.

No, as mentioned elsewhere, individual values might be quite a bit larger than 10 or 20MB. I think something along the lines of 100MB would be a more reasonable maximum, though per request it raises the question of how much a DoS protection this actually is.

I've updated the max to 1GB per Matt's recommendation. We get basic protection with this but I could spent some time on this and propose a coherent strategy.

ldk-reviews-bot · 2025-12-16T22:44:23Z

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

ldk-reviews-bot · 2025-12-18T22:45:03Z

🔔 2nd 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.

ldk-reviews-bot · 2025-12-22T00:01:02Z

🔔 3rd 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.

ldk-reviews-bot · 2025-12-24T00:01:45Z

🔔 4th 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.

ldk-reviews-bot · 2025-12-27T00:00:47Z

🔔 5th 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.

ldk-reviews-bot · 2025-12-29T00:01:37Z

🔔 6th 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.

ldk-reviews-bot · 2025-12-31T00:02:30Z

🔔 7th 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.

ldk-reviews-bot · 2026-01-03T00:00:33Z

🔔 8th 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.

ldk-reviews-bot · 2026-01-05T00:01:22Z

🔔 9th 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.

tnull

Can we also add some test coverage to ensure the database / backends actually support the configured maximum values, i.e., that we're not otherwise limited somehow?

rust/server/src/vss_service.rs

tnull · 2026-01-05T13:51:27Z

rust/server/src/vss_service.rs

 use std::pin::Pin;
 use std::sync::Arc;

+const MAXIMUM_REQUEST_BODY_SIZE: u16 = 65_535;


No, as mentioned elsewhere, individual values might be quite a bit larger than 10 or 20MB. I think something along the lines of 100MB would be a more reasonable maximum, though per request it raises the question of how much a DoS protection this actually is.

rust/server/src/vss_service.rs

ldk-reviews-bot · 2026-01-07T00:02:16Z

🔔 10th 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.

ldk-reviews-bot · 2026-01-10T00:01:02Z

🔔 11th 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.

enigbe · 2026-01-10T23:15:58Z

Thanks for the review @tnull and @TheBlueMatt. I have updated the maximum request body size and added a test to check that our backend(s) support the configured value. It's ready for another look when you have the time.

ldk-reviews-bot · 2026-01-12T00:01:40Z

🔔 12th 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.

tnull

Generally looks good, mod some comments.

As noted above already, I do wonder how much we're gaining at all we we're limiting to 1GB per request. In any case, probably better than nothing. So generally concept ACK from my side, but I'll leave it to @tankyleo to decide.

rust/docker-compose.yml

rust/server/src/vss_service.rs

TheBlueMatt · 2026-01-13T14:15:51Z

I mean at least it avoids someone being able to OOM us by just sending us trash 🤷‍♂️ Of course VSS exposed directly would probably still be OOM'able with a few parallel requests, but hopefully a decent frontend proxy can limit the impact by buffering the requests first.

ldk-reviews-bot · 2026-01-14T00:02:23Z

🔔 13th 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.

tankyleo

Thank your for the PR sorry for the delay on my part, a few bits, looking good !

rust/docker-compose.yml

rust/api/src/kv_store_tests.rs

rust/server/src/vss_service.rs

enigbe · 2026-01-16T06:54:21Z

Hi @tankyleo
Thanks for the review. I've addressed concerns raised and rebased. This is good for another look.

rust/server/src/util/config.rs

rust/server/src/vss_service.rs

rust/server/src/main.rs

rust/server/vss-server-config.toml

tankyleo

Thanks a few more nits below, feel free to squash the fixups

rust/server/src/util/config.rs

rust/server/src/main.rs

rust/server/vss-server-config.toml

enigbe · 2026-01-27T10:55:15Z

Hi @tankyleo,
I've rebased, squashed the fixups, and addressed the remaining comments. It's ready for another look.

tankyleo

LGTM two nits, good to consolidate everything into two commits, one for the docker file, and one for the max_request_body_size feature.

As with the other PR, please remove the prefixes, and capitalize the first word of the commit message.

rust/impls/src/postgres_store.rs

rust/server/vss-server-config.toml

enigbe

I've addressed the last nits. When you approve I'll squash the remaining fixup commit.

tankyleo · 2026-02-02T20:50:18Z

I've addressed the last nits. When you approve I'll squash the remaining fixup commit.

LGTM thank you ! Would you mind consolidating the max request body size feature in a single commit ? I would prefer one commit for the docker service, and one for the max request body size thanks.

enigbe · 2026-02-06T19:35:51Z

Would you mind consolidating the max request body size feature in a single commit ? I would prefer one commit for the docker service, and one for the max request body size thanks.

I've consolidated the commits as requested.
Thank you for reviewing!

Add VssServiceConfig to make the maximum request body size configurable through the server configuration file, with a hard limit of 1GB. Additionally, add test coverage for 1GB maximum supported value size and verifies that storage backends can handle the configured maximum value size.

tankyleo

Thank you for the ping, appreciate your patience I am now back to focusing on VSS. A quick comment for now, will do a more comprehensive review tomorrow

tankyleo · 2026-02-26T23:47:16Z

rust/server/src/util/config.rs

-	let bind_address_env = read_env(BIND_ADDR_VAR)?;
+	let (bind_address_config, max_request_body_size_config) = match server_config {
+		Some(c) => (c.bind_address, c.max_request_body_size),
+		None => (None, None),
+	};
+
+	let bind_address_env = read_env(BIND_ADDR_VAR)?
+		.map(|addr| {
+			addr.parse().map_err(|e| {
+				format!("Unable to parse the bind address environment variable: {}", e)
+			})
+		})
+		.transpose()?;


Can you double check this section ? Looks like the rebase reverted some code to how it was before. We don't need to parse BIND_ADDR_VAR into an Option<String>, we can use the original let bind_address_env = read_env(BIND_ADDR_VAR)?; thank you

tankyleo

Made another pass, overall it's nearly ready, just a few more nits that just popped up thank you for continuing the work on this PR. Please amend all the fixes into the top commit.

tankyleo · 2026-03-02T04:15:36Z

rust/impls/src/postgres_store.rs

+
+		// Construct entry that's for a field that's the maximum size of a non-"large_object" object
+		let large_value = vec![0u8; MAXIMUM_SUPPORTED_VALUE_SIZE - PROTOCOL_OVERHEAD_MARGIN];
+		let kv = KeyValue { key: "k1".into(), version: 0, value: Bytes::from(large_value) };


We'll need to clone large_value here

tankyleo · 2026-03-02T04:15:58Z

rust/impls/src/postgres_store.rs

+				resp_kv.value.len(),
+				MAXIMUM_SUPPORTED_VALUE_SIZE - PROTOCOL_OVERHEAD_MARGIN
+			);
+			assert!(resp_kv.value.iter().all(|&b| b == 0));


Let's do this here: assert_eq!(resp_kv.value, large_value);

tankyleo · 2026-03-02T04:17:58Z

rust/server/src/main.rs

+	let vss_service_config = match &config.max_request_body_size {
+		Some(size) => match VssServiceConfig::new(*size) {


usize is Copy, so we can do match config.max_request_body_size and VssServiceConfig::new(size)

tankyleo · 2026-03-02T04:24:21Z

rust/server/src/util/config.rs

+	let max_request_body_size_env = read_env(MAX_REQUEST_BODY_SIZE_VAR)?
+		.map(|mrbs| {
+			mrbs.parse::<usize>().map_err(|e| {
+				format!("Unable to parse the maximum request body size environment variable: {}", e)
+			})
+		})
+		.transpose()?;
+	let max_request_body_size = max_request_body_size_env.or(max_request_body_size_config);
+


nit: This is a little out of place here, let's put this after the let bind_address statement further above, thank you appreciate it

This has been moved up.

tankyleo · 2026-03-02T04:26:04Z

rust/server/src/vss_service.rs

+			return Ok(Response::builder()
+				.status(StatusCode::PAYLOAD_TOO_LARGE)
+				.body(Full::new(Bytes::from("Request body too large")))
+				.unwrap());


For consistency, let's also add // unwrap safety: body only errors when previous chained calls failed. right above this line, like we do for the other body calls below.

I've added the comment here.

Addresses the following: - read bind address correctly, - add appropriate unwrap documentation, - drop ref to mrbs because usize impls clone, - clone large object, and - assert_eq(Byte, Vec<u8>): because Bytes impls PartialEq with Vec<u8>, we can compare them directly.

enigbe · 2026-03-03T21:23:36Z

Thanks again for the review @tankyleo
I've addressed the latest comments in 5cef3b6.
Once you approve, I'll squash the commit.

ldk-reviews-bot requested a review from tankyleo December 14, 2025 22:43

enigbe force-pushed the 2025-12-bound-incoming-request-and-postgres-service branch from 4bfb403 to a163ae7 Compare December 14, 2025 22:45

tnull reviewed Dec 15, 2025

View reviewed changes

enigbe force-pushed the 2025-12-bound-incoming-request-and-postgres-service branch from 5d391cc to fbdd957 Compare December 16, 2025 08:36

tnull reviewed Jan 5, 2026

View reviewed changes

TheBlueMatt reviewed Jan 6, 2026

View reviewed changes

rust/server/src/vss_service.rs Outdated Show resolved Hide resolved

enigbe force-pushed the 2025-12-bound-incoming-request-and-postgres-service branch 2 times, most recently from 97b6b25 to 93faf38 Compare January 10, 2026 19:14

tnull self-requested a review January 12, 2026 14:37

tnull reviewed Jan 13, 2026

View reviewed changes

rust/docker-compose.yml Outdated Show resolved Hide resolved

rust/server/src/vss_service.rs Outdated Show resolved Hide resolved

enigbe force-pushed the 2025-12-bound-incoming-request-and-postgres-service branch 2 times, most recently from 15417ec to 7121dcc Compare January 13, 2026 23:21

tankyleo reviewed Jan 14, 2026

View reviewed changes

rust/docker-compose.yml Outdated Show resolved Hide resolved

rust/api/src/kv_store_tests.rs Outdated Show resolved Hide resolved

rust/server/src/vss_service.rs Outdated Show resolved Hide resolved

enigbe force-pushed the 2025-12-bound-incoming-request-and-postgres-service branch from 7121dcc to f061dc1 Compare January 15, 2026 22:58

enigbe force-pushed the 2025-12-bound-incoming-request-and-postgres-service branch from f061dc1 to 8a49df9 Compare January 15, 2026 23:13

tankyleo self-requested a review January 17, 2026 03:11

tankyleo reviewed Jan 17, 2026

View reviewed changes

rust/server/src/util/config.rs Outdated Show resolved Hide resolved

tankyleo reviewed Jan 17, 2026

View reviewed changes

rust/server/src/vss_service.rs Outdated Show resolved Hide resolved

rust/server/src/vss_service.rs Outdated Show resolved Hide resolved

tankyleo reviewed Jan 17, 2026

View reviewed changes

rust/server/src/main.rs Outdated Show resolved Hide resolved

tankyleo reviewed Jan 17, 2026

View reviewed changes

rust/server/vss-server-config.toml Outdated Show resolved Hide resolved

enigbe force-pushed the 2025-12-bound-incoming-request-and-postgres-service branch 2 times, most recently from 564f34a to 5d0d841 Compare January 23, 2026 09:06

tankyleo reviewed Jan 26, 2026

View reviewed changes

rust/server/vss-server-config.toml Outdated Show resolved Hide resolved

enigbe force-pushed the 2025-12-bound-incoming-request-and-postgres-service branch from 5d0d841 to 04a9213 Compare January 27, 2026 10:29

tankyleo reviewed Jan 27, 2026

View reviewed changes

rust/impls/src/postgres_store.rs Outdated Show resolved Hide resolved

rust/server/vss-server-config.toml Outdated Show resolved Hide resolved

enigbe force-pushed the 2025-12-bound-incoming-request-and-postgres-service branch from 04a9213 to e0ae2f9 Compare January 30, 2026 05:31

enigbe commented Jan 30, 2026

View reviewed changes

enigbe force-pushed the 2025-12-bound-incoming-request-and-postgres-service branch from e0ae2f9 to c16f96c Compare February 6, 2026 18:32

enigbe added 2 commits February 23, 2026 19:09

Add postgres docker service

605168a

enigbe force-pushed the 2025-12-bound-incoming-request-and-postgres-service branch from c16f96c to 2963c8d Compare February 23, 2026 18:38

enigbe changed the title ~~feat: bound incoming request and add postgres service~~ Bound incoming request and add postgres service Feb 25, 2026

enigbe requested a review from tankyleo February 25, 2026 12:35

tankyleo reviewed Feb 26, 2026

View reviewed changes

tankyleo reviewed Mar 2, 2026

View reviewed changes

enigbe force-pushed the 2025-12-bound-incoming-request-and-postgres-service branch from 7943877 to 5cef3b6 Compare March 3, 2026 20:53

		let vss_service_config = match &config.max_request_body_size {
		Some(size) => match VssServiceConfig::new(*size) {

Conversation

enigbe commented Dec 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What this PR does

Uh oh!

ldk-reviews-bot commented Dec 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

ldk-reviews-bot commented Dec 16, 2025

Uh oh!

ldk-reviews-bot commented Dec 18, 2025

Uh oh!

ldk-reviews-bot commented Dec 22, 2025

Uh oh!

ldk-reviews-bot commented Dec 24, 2025

Uh oh!

ldk-reviews-bot commented Dec 27, 2025

Uh oh!

ldk-reviews-bot commented Dec 29, 2025

Uh oh!

ldk-reviews-bot commented Dec 31, 2025

Uh oh!

ldk-reviews-bot commented Jan 3, 2026

Uh oh!

ldk-reviews-bot commented Jan 5, 2026

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!

Uh oh!

ldk-reviews-bot commented Jan 7, 2026

Uh oh!

ldk-reviews-bot commented Jan 10, 2026

Uh oh!

enigbe commented Jan 10, 2026

Uh oh!

ldk-reviews-bot commented Jan 12, 2026

Uh oh!

tnull left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

TheBlueMatt commented Jan 13, 2026

Uh oh!

ldk-reviews-bot commented Jan 14, 2026

Uh oh!

tankyleo left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

enigbe commented Jan 16, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

tankyleo left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

enigbe commented Dec 14, 2025 •

edited

Loading

ldk-reviews-bot commented Dec 14, 2025 •

edited

Loading

tankyleo commented Feb 2, 2026 •

edited

Loading

tankyleo left a comment •

edited

Loading

enigbe commented Mar 3, 2026 •

edited

Loading