Skip to content

Use bitreq::Url for LSPS5 webhook URLs#4566

Open
tnull wants to merge 2 commits intolightningdevkit:mainfrom
tnull:2026-04-bitreq-url
Open

Use bitreq::Url for LSPS5 webhook URLs#4566
tnull wants to merge 2 commits intolightningdevkit:mainfrom
tnull:2026-04-bitreq-url

Conversation

@tnull
Copy link
Copy Markdown
Contributor

@tnull tnull commented Apr 14, 2026

Replace the custom LSPS5 URL parser with bitreq::Url while keeping the LSPS5-specific HTTPS and length checks. This reduces bespoke parsing logic and aligns accepted webhook URLs with the HTTP client's URL handling.

Co-Authored-By: HAL 9000

@ldk-reviews-bot
Copy link
Copy Markdown

ldk-reviews-bot commented Apr 14, 2026

👋 Thanks for assigning @TheBlueMatt 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-claude-review-bot
Copy link
Copy Markdown
Collaborator

ldk-claude-review-bot commented Apr 14, 2026

I've thoroughly reviewed the entire PR diff, checked wire compatibility, the new Writeable implementations, Hash/Eq consistency, and all behavioral changes. Both issues from my prior review pass have been addressed in the current code.

No new issues found.

No issues found. Both previously flagged concerns have been resolved:

  1. Hash/Eq consistencyPartialEq is now manually implemented over as_str(), consistent with Hash.
  2. Writeable allocationself.0.as_str().write(writer) now delegates to the new impl Writeable for &str, which writes length + bytes directly with no heap allocation.

The impl Writeable for &str addition in lightning/src/util/ser.rs is correct — it doesn't conflict with the existing blanket impl<'a, T: Writeable> Writeable for &'a T because str is unsized and doesn't implement Writeable. The String impl now delegates to &str, producing identical wire format.

Replace the custom LSPS5 URL parser with `bitreq::Url` while keeping the
LSPS5-specific HTTPS and length checks. This reduces bespoke parsing
logic and aligns accepted webhook URLs with the HTTP client's URL
handling.

Co-Authored-By: HAL 9000
Signed-off-by: Elias Rohrer <dev@tnull.de>
@tnull tnull force-pushed the 2026-04-bitreq-url branch from 2c1aa43 to e960edc Compare April 14, 2026 09:50
@TheBlueMatt TheBlueMatt removed the request for review from wpaulino April 14, 2026 13:38
Implement Writeable for &str so LSPS5 URL serialization can use the shared string serializer instead of duplicating the write-side length-prefix logic.

Co-Authored-By: HAL 9000
@tnull tnull requested a review from TheBlueMatt April 14, 2026 15:54
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.

4 participants