feat: bound incoming request and add postgres service #76

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

tankyleo · 2026-01-14T04:27:13Z

rust/server/src/vss_service.rs

+
+impl VssServiceConfig {
+	pub fn new(maximum_request_body_size: usize) -> Self {
+		Self { maximum_request_body_size: maximum_request_body_size.min(MAXIMUM_REQUEST_BODY_SIZE) }


Instead of silently taking the min here, I would prefer we abort startup in case the user sets a value greater than 1 GiB to avoid surprises.

Addressed this by adding a validation of this after loading configuration.

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.

tankyleo · 2026-01-17T03:38:56Z

rust/server/src/util/config.rs

+	let maximum_request_body_size = read_config(
+		maximum_request_body_size_env,
+		max_request_body_size_config,
+		"VSS server maximum request body size",
+		MAX_REQUEST_BODY_SIZE,
+	)?;


Let's not make this parameter mandatory. In case it's not set, let's set it to None in the Configuration struct.

rust/server/src/vss_service.rs

tankyleo · 2026-01-17T03:47:42Z

rust/server/src/vss_service.rs

 pub struct VssService {
 	store: Arc<dyn KvStore>,
 	authorizer: Arc<dyn Authorizer>,
+	config: Arc<VssServiceConfig>,


I'd prefer we remove the Arc<> here, and do #[derive(Clone, Copy)] on VssServiceConfig

tankyleo · 2026-01-17T03:53:07Z

rust/server/src/main.rs

+			Ok(config) => Arc::new(config),
+			Err(e) => {
+				eprintln!("Configuration validation error: {}", e);
+				return;


One more thing, let's do an std::process:exit(-1) here like we do elsewhere

tankyleo · 2026-01-17T03:59:12Z

rust/server/vss-server-config.toml

@@ -1,5 +1,6 @@
 [server_config]
 bind_address = "127.0.0.1:8080" # Optional in TOML, can be overridden by env var `VSS_BIND_ADDRESS`
+maximum_request_body_size = 1073741824    # Optional in TOML:  maximum request body size in bytes capped at 1 GB, can be overriden by env var 'VSS_MAX_REQUEST_BODY_SIZE'


Let's keep this commented out to highlight to people that this parameter is optional; we'll default to 1GiB in case it is not set anywhere.

tankyleo

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

tankyleo · 2026-01-26T22:46:35Z

rust/server/src/util/config.rs

+		"VSS server maximum request body size",
+		MAX_REQUEST_BODY_SIZE,
+	)
+	.ok();


here let's do a maximum_request_body_size_env.or(max_request_body_size_config); we shouldn't pass parameters for a function to create an error that we later ignore :)

tankyleo · 2026-01-26T22:48:33Z

rust/server/src/util/config.rs

 use std::net::SocketAddr;

 const BIND_ADDR_VAR: &str = "VSS_BIND_ADDRESS";
+const MAX_REQUEST_BODY_SIZE: &str = "VSS_MAX_REQUEST_BODY_SIZE";


Let's keep things consistent here, how about MAX_REQUEST_BODY_SIZE_VAR ? feel free to pick a shorter name

tankyleo · 2026-01-26T22:52:37Z

rust/server/src/util/config.rs

 // Encapsulates the result of reading both the environment variables and the config file.
 pub(crate) struct Configuration {
 	pub(crate) bind_address: SocketAddr,
+	pub(crate) maximum_request_body_size: Option<usize>,


nit: would you mind using max_request_body_size here and in ServerConfig ? would like to shorten the name if we can since it's user facing :)

tankyleo · 2026-01-26T22:57:50Z

rust/server/src/main.rs

 						Ok((stream, _)) => {
 							let io_stream = TokioIo::new(stream);
-							let vss_service = VssService::new(Arc::clone(&store), Arc::clone(&authorizer));
+							let vss_service = VssService::new(Arc::clone(&store), Arc::clone(&authorizer), vss_service_config.clone());


Let's add the Copy trait on VssServiceConfig so that we don't have to call clone here; VsServiceConfig is just a single usize at the moment, so it can be cheaply copied.

tankyleo · 2026-01-26T23:07:56Z

rust/server/vss-server-config.toml

+# Maximum request body size in bytes. Can be set here or be overridden by env var 'VSS_MAX_REQUEST_BODY_SIZE'
+# Defaults to 1 GB if unset.
+# maximum_request_body_size = 1073741824                                     


Here let people know the max value they can set here is 1GiB

tankyleo · 2026-01-26T23:31:39Z

rust/server/vss-server-config.toml

 bind_address = "127.0.0.1:8080" # Optional in TOML, can be overridden by env var `VSS_BIND_ADDRESS`
+# Maximum request body size in bytes. Can be set here or be overridden by env var 'VSS_MAX_REQUEST_BODY_SIZE'
+# Defaults to 1 GB if unset.
+# maximum_request_body_size = 1073741824                                     


Ah one more thing, please delete the trailing whitespace on this line thanks

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.

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.

tankyleo · 2026-01-27T22:20:56Z

rust/impls/src/postgres_store.rs

 }

 #[cfg(test)]
+


Should have caught this, we don't need an additional line here

tankyleo · 2026-01-27T22:22:28Z

rust/server/vss-server-config.toml

-
+# Maximum request body size in bytes. Can be set here or be overridden by env var 'VSS_MAX_REQUEST_BODY_SIZE'
+# Defaults to the maximum possible value of 1 GB if unset.
+# max_request_body_size = 1073741824                                     


hmm sounds like the trailing whitespace is still there ? feel free to use something like git diff --check to check

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

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

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

enigbe added 4 commits January 27, 2026 08:41

build: add postgres docker service

2a3adbd

feat: set maximum request body size to 1GB

dfd8171

fixup! feat: add request body size configuration

04a9213

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

feat: bound incoming request and add postgres service #76

Are you sure you want to change the base?

feat: bound incoming request and add postgres service #76

Uh oh!

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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

enigbe commented Jan 16, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

enigbe commented Dec 14, 2025 •

edited

Loading

ldk-reviews-bot commented Dec 14, 2025 •

edited

Loading