Switch `ldk-server-client` from `reqwest` to `bitreq` by Anyitechs · Pull Request #127 · lightningdevkit/ldk-server

Anyitechs · 2026-02-16T17:34:58Z

This will switch ldk-server-client from using the reqwest dep to using the ecosystem maintained bitreq crate.

Meanwhile, this will be in draft till rust-bitcoin/corepc#502 hopefully lands and released, so we can bring back ability to load custom root certificates.

Closes#119

ldk-reviews-bot · 2026-02-16T17:35:03Z

👋 Thanks for assigning @benthecarman 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.

ldk-reviews-bot · 2026-02-18T18:29:14Z

🔔 1st Reminder

Hey @benthecarman! 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.

benthecarman

We might want to hold off until ldk and ldk node have switched over for a clean break

benthecarman · 2026-02-18T18:31:30Z

- LdkServerError::new(InternalError, format!("Failed to read response body: {}", e))
-})?;
+ let status_code = response.status_code;
+ let is_success_status_code = || -> bool { (200..300).contains(&status_code) };


Why is this a function?

Was trying to mirror the is_success() function from reqwest and maintain a simple check like we were doing before decoding the response.

benthecarman · 2026-02-18T18:33:15Z

 };

 const APPLICATION_OCTET_STREAM: &str = "application/octet-stream";
+const MAX_CACHED_CONNECTIONS: usize = 10;


Is this what they're doing in corepc? I feel like this could be a lot lower. Not a huge deal though

Is this what they're doing in corepc?
Not sure, but the least recently used connection gets evicted when the cache is full. We can increase it (but not sure what the default is, so it will guide that as the docs didn't mention it)

Anyitechs · 2026-02-16T17:34:58Z

This will switch ldk-server-client from using the reqwest dep to using the ecosystem maintained bitreq crate.

Meanwhile, this will be in draft till rust-bitcoin/corepc#502 hopefully lands and released, so we can bring back ability to load custom root certificates.

Closes#119

ldk-reviews-bot · 2026-02-16T17:35:03Z

👋 Thanks for assigning @benthecarman 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.

ldk-reviews-bot · 2026-02-18T18:29:14Z

🔔 1st Reminder

Hey @benthecarman! 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.

benthecarman

We might want to hold off until ldk and ldk node have switched over for a clean break

benthecarman · 2026-02-18T18:31:30Z

- LdkServerError::new(InternalError, format!("Failed to read response body: {}", e))
-})?;
+ let status_code = response.status_code;
+ let is_success_status_code = || -> bool { (200..300).contains(&status_code) };


Why is this a function?

Was trying to mirror the is_success() function from reqwest and maintain a simple check like we were doing before decoding the response.

benthecarman · 2026-02-18T18:33:15Z

 };

 const APPLICATION_OCTET_STREAM: &str = "application/octet-stream";
+const MAX_CACHED_CONNECTIONS: usize = 10;


Is this what they're doing in corepc? I feel like this could be a lot lower. Not a huge deal though

Is this what they're doing in corepc?
Not sure, but the least recently used connection gets evicted when the cache is full. We can increase it (but not sure what the default is, so it will guide that as the docs didn't mention it)

Anyitechs · 2026-02-16T17:34:58Z

This will switch ldk-server-client from using the reqwest dep to using the ecosystem maintained bitreq crate.

Meanwhile, this will be in draft till rust-bitcoin/corepc#502 hopefully lands and released, so we can bring back ability to load custom root certificates.

Closes#119

ldk-reviews-bot · 2026-02-16T17:35:03Z

👋 Thanks for assigning @benthecarman 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.

ldk-reviews-bot · 2026-02-18T18:29:14Z

🔔 1st Reminder

Hey @benthecarman! 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.

benthecarman

We might want to hold off until ldk and ldk node have switched over for a clean break

benthecarman · 2026-02-18T18:31:30Z

- LdkServerError::new(InternalError, format!("Failed to read response body: {}", e))
-})?;
+ let status_code = response.status_code;
+ let is_success_status_code = || -> bool { (200..300).contains(&status_code) };


Why is this a function?

Was trying to mirror the is_success() function from reqwest and maintain a simple check like we were doing before decoding the response.

benthecarman · 2026-02-18T18:33:15Z

 };

 const APPLICATION_OCTET_STREAM: &str = "application/octet-stream";
+const MAX_CACHED_CONNECTIONS: usize = 10;


Is this what they're doing in corepc? I feel like this could be a lot lower. Not a huge deal though

Is this what they're doing in corepc?
Not sure, but the least recently used connection gets evicted when the cache is full. We can increase it (but not sure what the default is, so it will guide that as the docs didn't mention it)

Anyitechs · 2026-02-16T17:34:58Z

This will switch ldk-server-client from using the reqwest dep to using the ecosystem maintained bitreq crate.

Meanwhile, this will be in draft till rust-bitcoin/corepc#502 hopefully lands and released, so we can bring back ability to load custom root certificates.

Closes#119

ldk-reviews-bot · 2026-02-16T17:35:03Z

👋 Thanks for assigning @benthecarman 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.

ldk-reviews-bot · 2026-02-18T18:29:14Z

🔔 1st Reminder

Hey @benthecarman! 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.

benthecarman

We might want to hold off until ldk and ldk node have switched over for a clean break

benthecarman · 2026-02-18T18:31:30Z

- LdkServerError::new(InternalError, format!("Failed to read response body: {}", e))
-})?;
+ let status_code = response.status_code;
+ let is_success_status_code = || -> bool { (200..300).contains(&status_code) };


Why is this a function?

Was trying to mirror the is_success() function from reqwest and maintain a simple check like we were doing before decoding the response.

benthecarman · 2026-02-18T18:33:15Z

 };

 const APPLICATION_OCTET_STREAM: &str = "application/octet-stream";
+const MAX_CACHED_CONNECTIONS: usize = 10;


Is this what they're doing in corepc? I feel like this could be a lot lower. Not a huge deal though

Is this what they're doing in corepc?
Not sure, but the least recently used connection gets evicted when the cache is full. We can increase it (but not sure what the default is, so it will guide that as the docs didn't mention it)

Anyitechs · 2026-02-16T17:34:58Z

This will switch ldk-server-client from using the reqwest dep to using the ecosystem maintained bitreq crate.

Meanwhile, this will be in draft till rust-bitcoin/corepc#502 hopefully lands and released, so we can bring back ability to load custom root certificates.

Closes#119

ldk-reviews-bot · 2026-02-16T17:35:03Z

👋 Thanks for assigning @benthecarman 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.

ldk-reviews-bot · 2026-02-18T18:29:14Z

🔔 1st Reminder

Hey @benthecarman! 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.

benthecarman

We might want to hold off until ldk and ldk node have switched over for a clean break

benthecarman · 2026-02-18T18:31:30Z

- LdkServerError::new(InternalError, format!("Failed to read response body: {}", e))
-})?;
+ let status_code = response.status_code;
+ let is_success_status_code = || -> bool { (200..300).contains(&status_code) };


Why is this a function?

Was trying to mirror the is_success() function from reqwest and maintain a simple check like we were doing before decoding the response.

benthecarman · 2026-02-18T18:33:15Z

 };

 const APPLICATION_OCTET_STREAM: &str = "application/octet-stream";
+const MAX_CACHED_CONNECTIONS: usize = 10;


Is this what they're doing in corepc? I feel like this could be a lot lower. Not a huge deal though

Is this what they're doing in corepc?
Not sure, but the least recently used connection gets evicted when the cache is full. We can increase it (but not sure what the default is, so it will guide that as the docs didn't mention it)

Anyitechs · 2026-02-16T17:34:58Z

This will switch ldk-server-client from using the reqwest dep to using the ecosystem maintained bitreq crate.

Meanwhile, this will be in draft till rust-bitcoin/corepc#502 hopefully lands and released, so we can bring back ability to load custom root certificates.

Closes#119

ldk-reviews-bot · 2026-02-16T17:35:03Z

👋 Thanks for assigning @benthecarman 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.

ldk-reviews-bot · 2026-02-18T18:29:14Z

🔔 1st Reminder

Hey @benthecarman! 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.

benthecarman

We might want to hold off until ldk and ldk node have switched over for a clean break

benthecarman · 2026-02-18T18:31:30Z

- LdkServerError::new(InternalError, format!("Failed to read response body: {}", e))
-})?;
+ let status_code = response.status_code;
+ let is_success_status_code = || -> bool { (200..300).contains(&status_code) };


Why is this a function?

Was trying to mirror the is_success() function from reqwest and maintain a simple check like we were doing before decoding the response.

benthecarman · 2026-02-18T18:33:15Z

 };

 const APPLICATION_OCTET_STREAM: &str = "application/octet-stream";
+const MAX_CACHED_CONNECTIONS: usize = 10;


Is this what they're doing in corepc? I feel like this could be a lot lower. Not a huge deal though

Is this what they're doing in corepc?
Not sure, but the least recently used connection gets evicted when the cache is full. We can increase it (but not sure what the default is, so it will guide that as the docs didn't mention it)

Anyitechs · 2026-02-16T17:34:58Z

This will switch ldk-server-client from using the reqwest dep to using the ecosystem maintained bitreq crate.

Meanwhile, this will be in draft till rust-bitcoin/corepc#502 hopefully lands and released, so we can bring back ability to load custom root certificates.

Closes#119

ldk-reviews-bot · 2026-02-16T17:35:03Z

👋 Thanks for assigning @benthecarman 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.

ldk-reviews-bot · 2026-02-18T18:29:14Z

🔔 1st Reminder

Hey @benthecarman! 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.

benthecarman

We might want to hold off until ldk and ldk node have switched over for a clean break

benthecarman · 2026-02-18T18:31:30Z

- LdkServerError::new(InternalError, format!("Failed to read response body: {}", e))
-})?;
+ let status_code = response.status_code;
+ let is_success_status_code = || -> bool { (200..300).contains(&status_code) };


Why is this a function?

Was trying to mirror the is_success() function from reqwest and maintain a simple check like we were doing before decoding the response.

benthecarman · 2026-02-18T18:33:15Z

 };

 const APPLICATION_OCTET_STREAM: &str = "application/octet-stream";
+const MAX_CACHED_CONNECTIONS: usize = 10;


Is this what they're doing in corepc? I feel like this could be a lot lower. Not a huge deal though

Is this what they're doing in corepc?
Not sure, but the least recently used connection gets evicted when the cache is full. We can increase it (but not sure what the default is, so it will guide that as the docs didn't mention it)

Anyitechs · 2026-02-16T17:34:58Z

This will switch ldk-server-client from using the reqwest dep to using the ecosystem maintained bitreq crate.

Meanwhile, this will be in draft till rust-bitcoin/corepc#502 hopefully lands and released, so we can bring back ability to load custom root certificates.

Closes#119

ldk-reviews-bot · 2026-02-16T17:35:03Z

👋 Thanks for assigning @benthecarman 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.

ldk-reviews-bot · 2026-02-18T18:29:14Z

🔔 1st Reminder

Hey @benthecarman! 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.

benthecarman

We might want to hold off until ldk and ldk node have switched over for a clean break

benthecarman · 2026-02-18T18:31:30Z

- LdkServerError::new(InternalError, format!("Failed to read response body: {}", e))
-})?;
+ let status_code = response.status_code;
+ let is_success_status_code = || -> bool { (200..300).contains(&status_code) };


Why is this a function?

Was trying to mirror the is_success() function from reqwest and maintain a simple check like we were doing before decoding the response.

benthecarman · 2026-02-18T18:33:15Z

 };

 const APPLICATION_OCTET_STREAM: &str = "application/octet-stream";
+const MAX_CACHED_CONNECTIONS: usize = 10;


Is this what they're doing in corepc? I feel like this could be a lot lower. Not a huge deal though

Is this what they're doing in corepc?
Not sure, but the least recently used connection gets evicted when the cache is full. We can increase it (but not sure what the default is, so it will guide that as the docs didn't mention it)

benthecarman self-requested a review February 16, 2026 18:28

benthecarman reviewed Feb 18, 2026
View reviewed changes