Skip to content

Conversation

@enigbe
Copy link
Contributor

@enigbe enigbe commented Dec 14, 2025

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]
    • Defaults to 10 MB if not specified; capped at 20 MB

@ldk-reviews-bot
Copy link

ldk-reviews-bot commented Dec 14, 2025

I've assigned @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.

@enigbe enigbe force-pushed the 2025-12-bound-incoming-request-and-postgres-service branch from 4bfb403 to a163ae7 Compare December 14, 2025 22:45
use std::pin::Pin;
use std::sync::Arc;

const MAXIMUM_REQUEST_BODY_SIZE: u16 = 65_535;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

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

🔔 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
Copy link

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants