Switch from reqwest to bitreq HTTP client
#56
Conversation
Replace the `reqwest` dependency with `bitreq`, a lighter HTTP(s) client that reduces code bloat and dependency overhead. Key changes: - Use `bitreq::Client` for connection pooling and reuse - Update all HTTP request handling to use bitreq's API - Remove `reqwest::header::HeaderMap` in favor of `HashMap<String, String>` - Simplify `LnurlAuthToJwtProvider::new()` to no longer return a `Result` - Use `serde_json::from_slice()` directly for JSON parsing - Build script uses bitreq's blocking `send()` method Co-Authored-By: HAL 9000 Signed-off-by: Elias Rohrer <dev@tnull.de>
|
👋 Thanks for assigning @tankyleo as a reviewer! |
tnull
left a comment
tnull
left a comment
There was a problem hiding this comment.
Honestly not sure why the idna crate suddenly started failing to build under 1.75.0, and we aim to soon replace the whole url dependency with our own Url type, but for now we can also just fix the issue by bumping MSRV to 1.85, since LDK Node already requires that anyways and we aim too soon also update LDK to that.
Wait, what? This is news to me. Can we just remove the cursed |
No, Matt, we discussed this plan for hours at the Rust Summit. We landed on that most ecosystem crates (BDK, etc) would update ~right away given their release cycles, but that LDK would defer bumping until after Ubuntu 2604 is out, which will of course happen this April.
Yeah, working on it as we speak. Just did a |
Right, April is a ways off though and we likely don't want to bump the day that a new release comes out (I'm kinda tired of us rushing to bump the day something comes out for the sake of it without a specific motivating rust feature, at least now that we've stripped out the stupid HTTP deps that keep causing dep-MSRV bumps), and I thought we wanted to upstream long before then lightningdevkit/rust-lightning#4323 |
| const APPLICATION_OCTET_STREAM: &str = "application/octet-stream"; | ||
| const DEFAULT_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(10); | ||
| const DEFAULT_TIMEOUT_SECS: u64 = 10; | ||
| const MAX_RESPONSE_BODY_SIZE: usize = 500 * 1024 * 1024; // 500 MiB |
There was a problem hiding this comment.
Now bumped this considerably, as of course 10MB would be much too small. Let me know if you have an opinion on a better value here @TheBlueMatt @tankyleo
There was a problem hiding this comment.
Mmm, yea, good question. On the server side we defaulted to 1G because its postgres' limit. Not that "just because its postgres' limit" is a good reason to do anything, though. 500 seems fine enough to me, honestly, though. As long as its documented I don't feel super strongly.
As 10 MB is def to small
Signed-off-by: Elias Rohrer <dev@tnull.de>
tnull
force-pushed
the
2026-01-switch-to-bitreq
branch
from
15114d4 to
ef17d83
Compare
3 participants
Replace the
reqwestdependency withbitreq, co-Authored-By: HAL 9000.