Skip to content

Add server-side events and html UI [PoC] - #171

Draft
joostjager wants to merge 4 commits into
lightningdevkit:mainfrom
joostjager:sse
Draft

Add server-side events and html UI [PoC]#171
joostjager wants to merge 4 commits into
lightningdevkit:mainfrom
joostjager:sse

Conversation

@joostjager

Copy link
Copy Markdown
Contributor
image

Based on #168

benthecarmanand others added 3 commits March 23, 2026 19:50
Protobuf added complexity without much benefit for our use case — the
binary encoding is opaque, hard to debug with standard HTTP tools, and
requires proto toolchain maintenance. JSON is human-readable, widely
supported, and sufficient for our throughput needs.
This removes prost and all .proto files entirely, renaming the
ldk-server-protos crate to ldk-server-json-models. Types are rewritten
as hand-written Rust structs and enums with serde derives rather than
prost-generated code. Fixed-size byte fields (hashes, channel IDs,
public keys) use [u8; 32] and [u8; 33] with hex serde instead of
String, giving type safety at the model layer.
Several proto-era patterns are cleaned up: wrapper structs that only
existed because protobuf wraps oneof in a message are removed, fields
that were Option only because proto message fields are nullable are
made required where the server always provides them, and the
EventEnvelope wrapper is dropped in favor of using Event directly.
Storage namespaces are changed from ("payments", "") to
("ldk-server", "payments") so existing protobuf-encoded data is
silently ignored rather than failing to deserialize, avoiding the
need for migration code or manual database wipes.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Remove the RabbitMQ-based EventPublisher trait and lapin dependency
in favor of a built-in SSE streaming endpoint. Events are now
delivered directly to clients over HTTP/TLS via a /Subscribe
endpoint, eliminating the need for an external message broker.
The server uses a tokio broadcast channel internally. The new
SseBody type implements hyper::body::Body to stream events as
JSON in SSE format. Event publishing becomes synchronous and
fire-and-forget when no subscribers are connected.
Add a subscribe command to the CLI that connects to the SSE
endpoint and prints each event as a JSON line to stdout. The
client library exposes a typed async event stream via
LdkServerClient::subscribe().
E2E tests use CliEventConsumer which spawns the CLI subscribe
command as a child process, replacing the previous raw TLS/SSE
consumer and RabbitMQ consumer.
AI tools were used in preparing this commit.
Add CORS headers to all server responses and handle OPTIONS
preflight requests, enabling browser-based clients to connect
directly to ldk-server.
Include a single-file web UI (index.html) that demonstrates
connecting to the JSON API and SSE event stream from the browser
using @microsoft/fetch-event-source. The UI shows node info,
lists peers with keysend buttons, supports BOLT11 payments, and
displays the live event stream.
AI tools were used in preparing this commit.
@ldk-reviews-bot

Copy link
Copy Markdown

👋 Hi! I see this is a draft PR.
I'll wait to assign reviewers until you mark it as ready for review.
Just convert it out of draft status when you're ready for review!

Full::new(Bytes::new()).boxed()
}

fn with_cors_headers(mut response: ServiceResponse) -> ServiceResponse {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

we probably want this configurable

@benthecarmanbenthecarman left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

this is great overall!

[dependencies]
ldk-server-json-models = { path = "../ldk-server-json-models" }
reqwest = { version = "0.11.13", default-features = false, features = ["rustls-tls"] }
reqwest = { version = "0.11.13", default-features = false, features = ["rustls-tls", "stream"] }

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I know we are planning on moving to bitreq, is this something that it supports?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I don't believe we do, no.

let forwarded_payment_creation_time = SystemTime::now().duration_since(UNIX_EPOCH).expect("Time must be > 1970").as_secs() as i64;

match event_publisher.publish(
let _ = event_sender.send(

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

we should still handle the error properly

}
}

fn error_to_response(e: LdkServerError) -> ServiceResponse {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Wouldn't this be better as just a From impl? Then we can just do ?

pub const GRAPH_GET_CHANNEL_PATH: &str = "GraphGetChannel";
pub const GRAPH_LIST_NODES_PATH: &str = "GraphListNodes";
pub const GRAPH_GET_NODE_PATH: &str = "GraphGetNode";
pub const SUBSCRIBE_PATH: &str = "Subscribe";

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Curious on your guy's thoughts on adding something like Subscribe/Payment/<id> and then you just get the events for that id.

Could see this nice for being able to permission a client to only certain events. Also makes it so you can just have a single stream for a payment instead of needing to filter between all events.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Might be useful indeed. I would only add it after user demand though

Comment threade2e-tests/src/lib.rs
use lapin::types::FieldTable;
use lapin::{ConnectionProperties, ExchangeKind};
impl CliEventConsumer {
/// Start the CLI subscribe command and begin receiving events in the background.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

using the cli here seems a little overly complex but i guess thats more e2e so maybe that's correct.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

I was in doubt about that too. Saw cli was already used, so I thought, make it as e2e as possible...

Comment on lines +459 to +468
let payload = response.bytes().await.map_err(|e| {
LdkServerError::new(InternalError, format!("Failed to read response body: {}", e))
})?;
let error_response =
serde_json::from_slice::<ErrorResponse>(&payload).map_err(|e| {
LdkServerError::new(
JsonParseError,
format!("Failed to decode error response (status {}): {}", status, e),
)
})?;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

You should be able to do let error_response: ErrorResponse = response.json() to clean this up a bunch

Ok(c) => c,
Err(_) => break,
};
buffer.push_str(&String::from_utf8_lossy(&chunk));

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Is there any potential concern here with multiple byte characters being split between multiple chunks and causing errors here?

Err(_) => break,
};
buffer.push_str(&String::from_utf8_lossy(&chunk));
while let Some(pos) = buffer.find("\n\n") {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I know this is handling for the SSE format but would be good to have some comments here explaining

> {
use futures_util::StreamExt;
let url = format!("https://{}/{SUBSCRIBE_PATH}", self.base_url);
let auth_header = self.compute_auth_header(&[]);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

unrelated to this PR but realizing the auth header should probably commit to the endpoint too

Derive ToSchema on all request/response and domain types in
ldk-server-json-models, with #[schema(value_type = String)] overrides
for hex-serialized fields. Define all 36 API path operations in a new
openapi module using utoipa's path macro on stub functions, grouped
by tags (Node, Onchain, Bolt11, Bolt12, Channels, Payments, Peers,
Send, Graph, Crypto, Events). The spec is served at /openapi.json
without authentication, cached via LazyLock.
AI tools were used in preparing this commit.
Sign up for freeto 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

@joostjager@ldk-reviews-bot@tnull@benthecarman
, 'i'); if (__m === '*' || __re.test(location.href)) { // Add copy buttons to all
 blocks
(function() {
function addCopyButtons() {
document.querySelectorAll('pre code').forEach(function(codeBlock) {
if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;
codeBlock.parentElement.setAttribute('data-copy-added', 'true');
var btn = document.createElement('button');
btn.textContent = 'Copy';
btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';
btn.onmouseover = function() { this.style.opacity = '1'; };
btn.onmouseout = function() { this.style.opacity = '0.7'; };
btn.onclick = function() {
navigator.clipboard.writeText(codeBlock.textContent).then(function() {
btn.textContent = 'Copied!';
setTimeout(function() { btn.textContent = 'Copy'; }, 1500);
});
};
codeBlock.parentElement.style.position = 'relative';
codeBlock.parentElement.appendChild(btn);
});
}
addCopyButtons();
// Re-run on dynamic content
var observer = new MutationObserver(addCopyButtons);
observer.observe(document.body, { childList: true, subtree: true });
})();
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Add server-side events and html UI [PoC] by joostjager · Pull Request #171 · lightningdevkit/ldk-server · GitHub
Skip to content

Add server-side events and html UI [PoC] - #171

Draft
joostjager wants to merge 4 commits into
lightningdevkit:mainfrom
joostjager:sse
Draft

Add server-side events and html UI [PoC]#171
joostjager wants to merge 4 commits into
lightningdevkit:mainfrom
joostjager:sse

Conversation

@joostjager

Copy link
Copy Markdown
Contributor
image

Based on #168

benthecarmanand others added 3 commits March 23, 2026 19:50
Protobuf added complexity without much benefit for our use case — the
binary encoding is opaque, hard to debug with standard HTTP tools, and
requires proto toolchain maintenance. JSON is human-readable, widely
supported, and sufficient for our throughput needs.
This removes prost and all .proto files entirely, renaming the
ldk-server-protos crate to ldk-server-json-models. Types are rewritten
as hand-written Rust structs and enums with serde derives rather than
prost-generated code. Fixed-size byte fields (hashes, channel IDs,
public keys) use [u8; 32] and [u8; 33] with hex serde instead of
String, giving type safety at the model layer.
Several proto-era patterns are cleaned up: wrapper structs that only
existed because protobuf wraps oneof in a message are removed, fields
that were Option only because proto message fields are nullable are
made required where the server always provides them, and the
EventEnvelope wrapper is dropped in favor of using Event directly.
Storage namespaces are changed from ("payments", "") to
("ldk-server", "payments") so existing protobuf-encoded data is
silently ignored rather than failing to deserialize, avoiding the
need for migration code or manual database wipes.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Remove the RabbitMQ-based EventPublisher trait and lapin dependency
in favor of a built-in SSE streaming endpoint. Events are now
delivered directly to clients over HTTP/TLS via a /Subscribe
endpoint, eliminating the need for an external message broker.
The server uses a tokio broadcast channel internally. The new
SseBody type implements hyper::body::Body to stream events as
JSON in SSE format. Event publishing becomes synchronous and
fire-and-forget when no subscribers are connected.
Add a subscribe command to the CLI that connects to the SSE
endpoint and prints each event as a JSON line to stdout. The
client library exposes a typed async event stream via
LdkServerClient::subscribe().
E2E tests use CliEventConsumer which spawns the CLI subscribe
command as a child process, replacing the previous raw TLS/SSE
consumer and RabbitMQ consumer.
AI tools were used in preparing this commit.
Add CORS headers to all server responses and handle OPTIONS
preflight requests, enabling browser-based clients to connect
directly to ldk-server.
Include a single-file web UI (index.html) that demonstrates
connecting to the JSON API and SSE event stream from the browser
using @microsoft/fetch-event-source. The UI shows node info,
lists peers with keysend buttons, supports BOLT11 payments, and
displays the live event stream.
AI tools were used in preparing this commit.
@ldk-reviews-bot

Copy link
Copy Markdown

👋 Hi! I see this is a draft PR.
I'll wait to assign reviewers until you mark it as ready for review.
Just convert it out of draft status when you're ready for review!

Full::new(Bytes::new()).boxed()
}

fn with_cors_headers(mut response: ServiceResponse) -> ServiceResponse {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

we probably want this configurable

@benthecarmanbenthecarman left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

this is great overall!

[dependencies]
ldk-server-json-models = { path = "../ldk-server-json-models" }
reqwest = { version = "0.11.13", default-features = false, features = ["rustls-tls"] }
reqwest = { version = "0.11.13", default-features = false, features = ["rustls-tls", "stream"] }

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I know we are planning on moving to bitreq, is this something that it supports?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I don't believe we do, no.

let forwarded_payment_creation_time = SystemTime::now().duration_since(UNIX_EPOCH).expect("Time must be > 1970").as_secs() as i64;

match event_publisher.publish(
let _ = event_sender.send(

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

we should still handle the error properly

}
}

fn error_to_response(e: LdkServerError) -> ServiceResponse {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Wouldn't this be better as just a From impl? Then we can just do ?

pub const GRAPH_GET_CHANNEL_PATH: &str = "GraphGetChannel";
pub const GRAPH_LIST_NODES_PATH: &str = "GraphListNodes";
pub const GRAPH_GET_NODE_PATH: &str = "GraphGetNode";
pub const SUBSCRIBE_PATH: &str = "Subscribe";

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Curious on your guy's thoughts on adding something like Subscribe/Payment/<id> and then you just get the events for that id.

Could see this nice for being able to permission a client to only certain events. Also makes it so you can just have a single stream for a payment instead of needing to filter between all events.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Might be useful indeed. I would only add it after user demand though

Comment threade2e-tests/src/lib.rs
use lapin::types::FieldTable;
use lapin::{ConnectionProperties, ExchangeKind};
impl CliEventConsumer {
/// Start the CLI subscribe command and begin receiving events in the background.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

using the cli here seems a little overly complex but i guess thats more e2e so maybe that's correct.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

I was in doubt about that too. Saw cli was already used, so I thought, make it as e2e as possible...

Comment on lines +459 to +468
let payload = response.bytes().await.map_err(|e| {
LdkServerError::new(InternalError, format!("Failed to read response body: {}", e))
})?;
let error_response =
serde_json::from_slice::<ErrorResponse>(&payload).map_err(|e| {
LdkServerError::new(
JsonParseError,
format!("Failed to decode error response (status {}): {}", status, e),
)
})?;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

You should be able to do let error_response: ErrorResponse = response.json() to clean this up a bunch

Ok(c) => c,
Err(_) => break,
};
buffer.push_str(&String::from_utf8_lossy(&chunk));

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Is there any potential concern here with multiple byte characters being split between multiple chunks and causing errors here?

Err(_) => break,
};
buffer.push_str(&String::from_utf8_lossy(&chunk));
while let Some(pos) = buffer.find("\n\n") {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I know this is handling for the SSE format but would be good to have some comments here explaining

> {
use futures_util::StreamExt;
let url = format!("https://{}/{SUBSCRIBE_PATH}", self.base_url);
let auth_header = self.compute_auth_header(&[]);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

unrelated to this PR but realizing the auth header should probably commit to the endpoint too

Derive ToSchema on all request/response and domain types in
ldk-server-json-models, with #[schema(value_type = String)] overrides
for hex-serialized fields. Define all 36 API path operations in a new
openapi module using utoipa's path macro on stub functions, grouped
by tags (Node, Onchain, Bolt11, Bolt12, Channels, Payments, Peers,
Send, Graph, Crypto, Events). The spec is served at /openapi.json
without authentication, cached via LazyLock.
AI tools were used in preparing this commit.
Sign up for freeto 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

@joostjager@ldk-reviews-bot@tnull@benthecarman
, 'i'); if (__m === '*' || __re.test(location.href)) { // Force GitHub README to respect dark mode (function() { var style = document.createElement('style'); style.textContent = ' .markdown-body { color-scheme: dark light; } .markdown-body pre { background: #161b22 !important; } .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; } .markdown-body table th, .markdown-body table td { border-color: #30363d !important; } .markdown-body img { background: #0d1117; } .markdown-body blockquote { border-left-color: #8b949e; } .markdown-body hr { border-color: #30363d; } '; document.head.appendChild(style); })(); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' Add server-side events and html UI [PoC] by joostjager · Pull Request #171 · lightningdevkit/ldk-server · GitHub
Skip to content

Add server-side events and html UI [PoC] - #171

Draft
joostjager wants to merge 4 commits into
lightningdevkit:mainfrom
joostjager:sse
Draft

Add server-side events and html UI [PoC]#171
joostjager wants to merge 4 commits into
lightningdevkit:mainfrom
joostjager:sse

Conversation

@joostjager

Copy link
Copy Markdown
Contributor
image

Based on #168

benthecarmanand others added 3 commits March 23, 2026 19:50
Protobuf added complexity without much benefit for our use case — the
binary encoding is opaque, hard to debug with standard HTTP tools, and
requires proto toolchain maintenance. JSON is human-readable, widely
supported, and sufficient for our throughput needs.
This removes prost and all .proto files entirely, renaming the
ldk-server-protos crate to ldk-server-json-models. Types are rewritten
as hand-written Rust structs and enums with serde derives rather than
prost-generated code. Fixed-size byte fields (hashes, channel IDs,
public keys) use [u8; 32] and [u8; 33] with hex serde instead of
String, giving type safety at the model layer.
Several proto-era patterns are cleaned up: wrapper structs that only
existed because protobuf wraps oneof in a message are removed, fields
that were Option only because proto message fields are nullable are
made required where the server always provides them, and the
EventEnvelope wrapper is dropped in favor of using Event directly.
Storage namespaces are changed from ("payments", "") to
("ldk-server", "payments") so existing protobuf-encoded data is
silently ignored rather than failing to deserialize, avoiding the
need for migration code or manual database wipes.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Remove the RabbitMQ-based EventPublisher trait and lapin dependency
in favor of a built-in SSE streaming endpoint. Events are now
delivered directly to clients over HTTP/TLS via a /Subscribe
endpoint, eliminating the need for an external message broker.
The server uses a tokio broadcast channel internally. The new
SseBody type implements hyper::body::Body to stream events as
JSON in SSE format. Event publishing becomes synchronous and
fire-and-forget when no subscribers are connected.
Add a subscribe command to the CLI that connects to the SSE
endpoint and prints each event as a JSON line to stdout. The
client library exposes a typed async event stream via
LdkServerClient::subscribe().
E2E tests use CliEventConsumer which spawns the CLI subscribe
command as a child process, replacing the previous raw TLS/SSE
consumer and RabbitMQ consumer.
AI tools were used in preparing this commit.
Add CORS headers to all server responses and handle OPTIONS
preflight requests, enabling browser-based clients to connect
directly to ldk-server.
Include a single-file web UI (index.html) that demonstrates
connecting to the JSON API and SSE event stream from the browser
using @microsoft/fetch-event-source. The UI shows node info,
lists peers with keysend buttons, supports BOLT11 payments, and
displays the live event stream.
AI tools were used in preparing this commit.
@ldk-reviews-bot

Copy link
Copy Markdown

👋 Hi! I see this is a draft PR.
I'll wait to assign reviewers until you mark it as ready for review.
Just convert it out of draft status when you're ready for review!

Full::new(Bytes::new()).boxed()
}

fn with_cors_headers(mut response: ServiceResponse) -> ServiceResponse {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

we probably want this configurable

@benthecarmanbenthecarman left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

this is great overall!

[dependencies]
ldk-server-json-models = { path = "../ldk-server-json-models" }
reqwest = { version = "0.11.13", default-features = false, features = ["rustls-tls"] }
reqwest = { version = "0.11.13", default-features = false, features = ["rustls-tls", "stream"] }

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I know we are planning on moving to bitreq, is this something that it supports?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I don't believe we do, no.

let forwarded_payment_creation_time = SystemTime::now().duration_since(UNIX_EPOCH).expect("Time must be > 1970").as_secs() as i64;

match event_publisher.publish(
let _ = event_sender.send(

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

we should still handle the error properly

}
}

fn error_to_response(e: LdkServerError) -> ServiceResponse {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Wouldn't this be better as just a From impl? Then we can just do ?

pub const GRAPH_GET_CHANNEL_PATH: &str = "GraphGetChannel";
pub const GRAPH_LIST_NODES_PATH: &str = "GraphListNodes";
pub const GRAPH_GET_NODE_PATH: &str = "GraphGetNode";
pub const SUBSCRIBE_PATH: &str = "Subscribe";

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Curious on your guy's thoughts on adding something like Subscribe/Payment/<id> and then you just get the events for that id.

Could see this nice for being able to permission a client to only certain events. Also makes it so you can just have a single stream for a payment instead of needing to filter between all events.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Might be useful indeed. I would only add it after user demand though

Comment threade2e-tests/src/lib.rs
use lapin::types::FieldTable;
use lapin::{ConnectionProperties, ExchangeKind};
impl CliEventConsumer {
/// Start the CLI subscribe command and begin receiving events in the background.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

using the cli here seems a little overly complex but i guess thats more e2e so maybe that's correct.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

I was in doubt about that too. Saw cli was already used, so I thought, make it as e2e as possible...

Comment on lines +459 to +468
let payload = response.bytes().await.map_err(|e| {
LdkServerError::new(InternalError, format!("Failed to read response body: {}", e))
})?;
let error_response =
serde_json::from_slice::<ErrorResponse>(&payload).map_err(|e| {
LdkServerError::new(
JsonParseError,
format!("Failed to decode error response (status {}): {}", status, e),
)
})?;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

You should be able to do let error_response: ErrorResponse = response.json() to clean this up a bunch

Ok(c) => c,
Err(_) => break,
};
buffer.push_str(&String::from_utf8_lossy(&chunk));

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Is there any potential concern here with multiple byte characters being split between multiple chunks and causing errors here?

Err(_) => break,
};
buffer.push_str(&String::from_utf8_lossy(&chunk));
while let Some(pos) = buffer.find("\n\n") {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I know this is handling for the SSE format but would be good to have some comments here explaining

> {
use futures_util::StreamExt;
let url = format!("https://{}/{SUBSCRIBE_PATH}", self.base_url);
let auth_header = self.compute_auth_header(&[]);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

unrelated to this PR but realizing the auth header should probably commit to the endpoint too

Derive ToSchema on all request/response and domain types in
ldk-server-json-models, with #[schema(value_type = String)] overrides
for hex-serialized fields. Define all 36 API path operations in a new
openapi module using utoipa's path macro on stub functions, grouped
by tags (Node, Onchain, Bolt11, Bolt12, Channels, Payments, Peers,
Send, Graph, Crypto, Events). The spec is served at /openapi.json
without authentication, cached via LazyLock.
AI tools were used in preparing this commit.
Sign up for freeto 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

@joostjager@ldk-reviews-bot@tnull@benthecarman
, 'i'); if (__m === '*' || __re.test(location.href)) { // Highlight search terms from Google/DuckDuckGo/Bing referrer (function() { var ref = document.referrer; var terms = []; if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) { var url = new URL(ref); var q = url.searchParams.get('q') || url.searchParams.get('p'); if (q) { terms = q.split(/\s+/).filter(function(t) { return t.length > 2; }); } } if (terms.length === 0) return; var style = document.createElement('style'); style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }'; document.head.appendChild(style); function highlight(node) { if (node.nodeType === 3) { // text node var text = node.textContent; var found = false; terms.forEach(function(term) { var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\]\\]/g, '\\') + ')', 'gi'); if (regex.test(text)) { found = true; var frag = document.createDocumentFragment(); var parts = text.split(regex); parts.forEach(function(part, i) { if (i % 2 === 0) { frag.appendChild(document.createTextNode(part)); } else { var span = document.createElement('span'); span.className = 'userscript-highlight'; span.textContent = part; frag.appendChild(span); } }); node.parentNode.replaceChild(frag, node); } }); } else if (node.nodeType === 1 && node.childNodes) { // element var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT']; if (!skipTags.includes(node.tagName)) { Array.from(node.childNodes).forEach(highlight); } } } highlight(document.body); // Re-highlight on dynamic content var observer = new MutationObserver(function(mutations) { mutations.forEach(function(m) { m.addedNodes.forEach(function(node) { if (node.nodeType === 1 || node.nodeType === 3) highlight(node); }); }); }); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' Add server-side events and html UI [PoC] by joostjager · Pull Request #171 · lightningdevkit/ldk-server · GitHub
Skip to content

Add server-side events and html UI [PoC] - #171

Draft
joostjager wants to merge 4 commits into
lightningdevkit:mainfrom
joostjager:sse
Draft

Add server-side events and html UI [PoC]#171
joostjager wants to merge 4 commits into
lightningdevkit:mainfrom
joostjager:sse

Conversation

@joostjager

Copy link
Copy Markdown
Contributor
image

Based on #168

benthecarmanand others added 3 commits March 23, 2026 19:50
Protobuf added complexity without much benefit for our use case — the
binary encoding is opaque, hard to debug with standard HTTP tools, and
requires proto toolchain maintenance. JSON is human-readable, widely
supported, and sufficient for our throughput needs.
This removes prost and all .proto files entirely, renaming the
ldk-server-protos crate to ldk-server-json-models. Types are rewritten
as hand-written Rust structs and enums with serde derives rather than
prost-generated code. Fixed-size byte fields (hashes, channel IDs,
public keys) use [u8; 32] and [u8; 33] with hex serde instead of
String, giving type safety at the model layer.
Several proto-era patterns are cleaned up: wrapper structs that only
existed because protobuf wraps oneof in a message are removed, fields
that were Option only because proto message fields are nullable are
made required where the server always provides them, and the
EventEnvelope wrapper is dropped in favor of using Event directly.
Storage namespaces are changed from ("payments", "") to
("ldk-server", "payments") so existing protobuf-encoded data is
silently ignored rather than failing to deserialize, avoiding the
need for migration code or manual database wipes.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Remove the RabbitMQ-based EventPublisher trait and lapin dependency
in favor of a built-in SSE streaming endpoint. Events are now
delivered directly to clients over HTTP/TLS via a /Subscribe
endpoint, eliminating the need for an external message broker.
The server uses a tokio broadcast channel internally. The new
SseBody type implements hyper::body::Body to stream events as
JSON in SSE format. Event publishing becomes synchronous and
fire-and-forget when no subscribers are connected.
Add a subscribe command to the CLI that connects to the SSE
endpoint and prints each event as a JSON line to stdout. The
client library exposes a typed async event stream via
LdkServerClient::subscribe().
E2E tests use CliEventConsumer which spawns the CLI subscribe
command as a child process, replacing the previous raw TLS/SSE
consumer and RabbitMQ consumer.
AI tools were used in preparing this commit.
Add CORS headers to all server responses and handle OPTIONS
preflight requests, enabling browser-based clients to connect
directly to ldk-server.
Include a single-file web UI (index.html) that demonstrates
connecting to the JSON API and SSE event stream from the browser
using @microsoft/fetch-event-source. The UI shows node info,
lists peers with keysend buttons, supports BOLT11 payments, and
displays the live event stream.
AI tools were used in preparing this commit.
@ldk-reviews-bot

Copy link
Copy Markdown

👋 Hi! I see this is a draft PR.
I'll wait to assign reviewers until you mark it as ready for review.
Just convert it out of draft status when you're ready for review!

Full::new(Bytes::new()).boxed()
}

fn with_cors_headers(mut response: ServiceResponse) -> ServiceResponse {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

we probably want this configurable

@benthecarmanbenthecarman left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

this is great overall!

[dependencies]
ldk-server-json-models = { path = "../ldk-server-json-models" }
reqwest = { version = "0.11.13", default-features = false, features = ["rustls-tls"] }
reqwest = { version = "0.11.13", default-features = false, features = ["rustls-tls", "stream"] }

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I know we are planning on moving to bitreq, is this something that it supports?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I don't believe we do, no.

let forwarded_payment_creation_time = SystemTime::now().duration_since(UNIX_EPOCH).expect("Time must be > 1970").as_secs() as i64;

match event_publisher.publish(
let _ = event_sender.send(

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

we should still handle the error properly

}
}

fn error_to_response(e: LdkServerError) -> ServiceResponse {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Wouldn't this be better as just a From impl? Then we can just do ?

pub const GRAPH_GET_CHANNEL_PATH: &str = "GraphGetChannel";
pub const GRAPH_LIST_NODES_PATH: &str = "GraphListNodes";
pub const GRAPH_GET_NODE_PATH: &str = "GraphGetNode";
pub const SUBSCRIBE_PATH: &str = "Subscribe";

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Curious on your guy's thoughts on adding something like Subscribe/Payment/<id> and then you just get the events for that id.

Could see this nice for being able to permission a client to only certain events. Also makes it so you can just have a single stream for a payment instead of needing to filter between all events.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Might be useful indeed. I would only add it after user demand though

Comment threade2e-tests/src/lib.rs
use lapin::types::FieldTable;
use lapin::{ConnectionProperties, ExchangeKind};
impl CliEventConsumer {
/// Start the CLI subscribe command and begin receiving events in the background.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

using the cli here seems a little overly complex but i guess thats more e2e so maybe that's correct.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

I was in doubt about that too. Saw cli was already used, so I thought, make it as e2e as possible...

Comment on lines +459 to +468
let payload = response.bytes().await.map_err(|e| {
LdkServerError::new(InternalError, format!("Failed to read response body: {}", e))
})?;
let error_response =
serde_json::from_slice::<ErrorResponse>(&payload).map_err(|e| {
LdkServerError::new(
JsonParseError,
format!("Failed to decode error response (status {}): {}", status, e),
)
})?;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

You should be able to do let error_response: ErrorResponse = response.json() to clean this up a bunch

Ok(c) => c,
Err(_) => break,
};
buffer.push_str(&String::from_utf8_lossy(&chunk));

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Is there any potential concern here with multiple byte characters being split between multiple chunks and causing errors here?

Err(_) => break,
};
buffer.push_str(&String::from_utf8_lossy(&chunk));
while let Some(pos) = buffer.find("\n\n") {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I know this is handling for the SSE format but would be good to have some comments here explaining

> {
use futures_util::StreamExt;
let url = format!("https://{}/{SUBSCRIBE_PATH}", self.base_url);
let auth_header = self.compute_auth_header(&[]);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

unrelated to this PR but realizing the auth header should probably commit to the endpoint too

Derive ToSchema on all request/response and domain types in
ldk-server-json-models, with #[schema(value_type = String)] overrides
for hex-serialized fields. Define all 36 API path operations in a new
openapi module using utoipa's path macro on stub functions, grouped
by tags (Node, Onchain, Bolt11, Bolt12, Channels, Payments, Peers,
Send, Graph, Crypto, Events). The spec is served at /openapi.json
without authentication, cached via LazyLock.
AI tools were used in preparing this commit.
Sign up for freeto 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

@joostjager@ldk-reviews-bot@tnull@benthecarman
, 'i'); if (__m === '*' || __re.test(location.href)) { // Strip utm_, fbclid, gclid, etc. from all links on page (function() { var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content', 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid', 'ref', 'ref_src', 'source', 'medium', 'campaign']; function cleanUrl(url) { try { var u = new URL(url, window.location.origin); var changed = false; trackingParams.forEach(function(p) { if (u.searchParams.has(p)) { u.searchParams.delete(p); changed = true; } }); return changed ? u.toString() : url; } catch (e) { return url; } } function cleanLinks() { document.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } cleanLinks(); var observer = new MutationObserver(function(mutations) { mutations.forEach(function(m) { m.addedNodes.forEach(function(node) { if (node.nodeType === 1) { if (node.tagName === 'A') cleanLinks(); node.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } }); }); }); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + ' Add server-side events and html UI [PoC] by joostjager · Pull Request #171 · lightningdevkit/ldk-server · GitHub
Skip to content

Add server-side events and html UI [PoC] - #171

Draft
joostjager wants to merge 4 commits into
lightningdevkit:mainfrom
joostjager:sse
Draft

Add server-side events and html UI [PoC]#171
joostjager wants to merge 4 commits into
lightningdevkit:mainfrom
joostjager:sse

Conversation

@joostjager

Copy link
Copy Markdown
Contributor
image

Based on #168

benthecarmanand others added 3 commits March 23, 2026 19:50
Protobuf added complexity without much benefit for our use case — the
binary encoding is opaque, hard to debug with standard HTTP tools, and
requires proto toolchain maintenance. JSON is human-readable, widely
supported, and sufficient for our throughput needs.
This removes prost and all .proto files entirely, renaming the
ldk-server-protos crate to ldk-server-json-models. Types are rewritten
as hand-written Rust structs and enums with serde derives rather than
prost-generated code. Fixed-size byte fields (hashes, channel IDs,
public keys) use [u8; 32] and [u8; 33] with hex serde instead of
String, giving type safety at the model layer.
Several proto-era patterns are cleaned up: wrapper structs that only
existed because protobuf wraps oneof in a message are removed, fields
that were Option only because proto message fields are nullable are
made required where the server always provides them, and the
EventEnvelope wrapper is dropped in favor of using Event directly.
Storage namespaces are changed from ("payments", "") to
("ldk-server", "payments") so existing protobuf-encoded data is
silently ignored rather than failing to deserialize, avoiding the
need for migration code or manual database wipes.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Remove the RabbitMQ-based EventPublisher trait and lapin dependency
in favor of a built-in SSE streaming endpoint. Events are now
delivered directly to clients over HTTP/TLS via a /Subscribe
endpoint, eliminating the need for an external message broker.
The server uses a tokio broadcast channel internally. The new
SseBody type implements hyper::body::Body to stream events as
JSON in SSE format. Event publishing becomes synchronous and
fire-and-forget when no subscribers are connected.
Add a subscribe command to the CLI that connects to the SSE
endpoint and prints each event as a JSON line to stdout. The
client library exposes a typed async event stream via
LdkServerClient::subscribe().
E2E tests use CliEventConsumer which spawns the CLI subscribe
command as a child process, replacing the previous raw TLS/SSE
consumer and RabbitMQ consumer.
AI tools were used in preparing this commit.
Add CORS headers to all server responses and handle OPTIONS
preflight requests, enabling browser-based clients to connect
directly to ldk-server.
Include a single-file web UI (index.html) that demonstrates
connecting to the JSON API and SSE event stream from the browser
using @microsoft/fetch-event-source. The UI shows node info,
lists peers with keysend buttons, supports BOLT11 payments, and
displays the live event stream.
AI tools were used in preparing this commit.
@ldk-reviews-bot

Copy link
Copy Markdown

👋 Hi! I see this is a draft PR.
I'll wait to assign reviewers until you mark it as ready for review.
Just convert it out of draft status when you're ready for review!

Full::new(Bytes::new()).boxed()
}

fn with_cors_headers(mut response: ServiceResponse) -> ServiceResponse {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

we probably want this configurable

@benthecarmanbenthecarman left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

this is great overall!

[dependencies]
ldk-server-json-models = { path = "../ldk-server-json-models" }
reqwest = { version = "0.11.13", default-features = false, features = ["rustls-tls"] }
reqwest = { version = "0.11.13", default-features = false, features = ["rustls-tls", "stream"] }

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I know we are planning on moving to bitreq, is this something that it supports?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I don't believe we do, no.

let forwarded_payment_creation_time = SystemTime::now().duration_since(UNIX_EPOCH).expect("Time must be > 1970").as_secs() as i64;

match event_publisher.publish(
let _ = event_sender.send(

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

we should still handle the error properly

}
}

fn error_to_response(e: LdkServerError) -> ServiceResponse {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Wouldn't this be better as just a From impl? Then we can just do ?

pub const GRAPH_GET_CHANNEL_PATH: &str = "GraphGetChannel";
pub const GRAPH_LIST_NODES_PATH: &str = "GraphListNodes";
pub const GRAPH_GET_NODE_PATH: &str = "GraphGetNode";
pub const SUBSCRIBE_PATH: &str = "Subscribe";

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Curious on your guy's thoughts on adding something like Subscribe/Payment/<id> and then you just get the events for that id.

Could see this nice for being able to permission a client to only certain events. Also makes it so you can just have a single stream for a payment instead of needing to filter between all events.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Might be useful indeed. I would only add it after user demand though

Comment threade2e-tests/src/lib.rs
use lapin::types::FieldTable;
use lapin::{ConnectionProperties, ExchangeKind};
impl CliEventConsumer {
/// Start the CLI subscribe command and begin receiving events in the background.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

using the cli here seems a little overly complex but i guess thats more e2e so maybe that's correct.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

I was in doubt about that too. Saw cli was already used, so I thought, make it as e2e as possible...

Comment on lines +459 to +468
let payload = response.bytes().await.map_err(|e| {
LdkServerError::new(InternalError, format!("Failed to read response body: {}", e))
})?;
let error_response =
serde_json::from_slice::<ErrorResponse>(&payload).map_err(|e| {
LdkServerError::new(
JsonParseError,
format!("Failed to decode error response (status {}): {}", status, e),
)
})?;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

You should be able to do let error_response: ErrorResponse = response.json() to clean this up a bunch

Ok(c) => c,
Err(_) => break,
};
buffer.push_str(&String::from_utf8_lossy(&chunk));

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Is there any potential concern here with multiple byte characters being split between multiple chunks and causing errors here?

Err(_) => break,
};
buffer.push_str(&String::from_utf8_lossy(&chunk));
while let Some(pos) = buffer.find("\n\n") {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I know this is handling for the SSE format but would be good to have some comments here explaining

> {
use futures_util::StreamExt;
let url = format!("https://{}/{SUBSCRIBE_PATH}", self.base_url);
let auth_header = self.compute_auth_header(&[]);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

unrelated to this PR but realizing the auth header should probably commit to the endpoint too

Derive ToSchema on all request/response and domain types in
ldk-server-json-models, with #[schema(value_type = String)] overrides
for hex-serialized fields. Define all 36 API path operations in a new
openapi module using utoipa's path macro on stub functions, grouped
by tags (Node, Onchain, Bolt11, Bolt12, Channels, Payments, Peers,
Send, Graph, Crypto, Events). The spec is served at /openapi.json
without authentication, cached via LazyLock.
AI tools were used in preparing this commit.
Sign up for freeto 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

@joostjager@ldk-reviews-bot@tnull@benthecarman
, 'i'); if (__m === '*' || __re.test(location.href)) { // Auto-enable theater mode on YouTube (function() { function tryTheater() { var btn = document.querySelector('button[aria-label="Theater mode"], ytd-player #player button[title="Theater mode"]'); if (btn && !btn.classList.contains('activated')) { btn.click(); } } // Try immediately tryTheater(); // Try after navigation (SPA) var lastUrl = location.href; setInterval(function() { if (location.href !== lastUrl) { lastUrl = location.href; setTimeout(tryTheater, 500); } }, 1000); // Also try on player load var observer = new MutationObserver(tryTheater); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' Add server-side events and html UI [PoC] by joostjager · Pull Request #171 · lightningdevkit/ldk-server · GitHub
Skip to content

Add server-side events and html UI [PoC] - #171

Draft
joostjager wants to merge 4 commits into
lightningdevkit:mainfrom
joostjager:sse
Draft

Add server-side events and html UI [PoC]#171
joostjager wants to merge 4 commits into
lightningdevkit:mainfrom
joostjager:sse

Conversation

@joostjager

Copy link
Copy Markdown
Contributor
image

Based on #168

benthecarmanand others added 3 commits March 23, 2026 19:50
Protobuf added complexity without much benefit for our use case — the
binary encoding is opaque, hard to debug with standard HTTP tools, and
requires proto toolchain maintenance. JSON is human-readable, widely
supported, and sufficient for our throughput needs.
This removes prost and all .proto files entirely, renaming the
ldk-server-protos crate to ldk-server-json-models. Types are rewritten
as hand-written Rust structs and enums with serde derives rather than
prost-generated code. Fixed-size byte fields (hashes, channel IDs,
public keys) use [u8; 32] and [u8; 33] with hex serde instead of
String, giving type safety at the model layer.
Several proto-era patterns are cleaned up: wrapper structs that only
existed because protobuf wraps oneof in a message are removed, fields
that were Option only because proto message fields are nullable are
made required where the server always provides them, and the
EventEnvelope wrapper is dropped in favor of using Event directly.
Storage namespaces are changed from ("payments", "") to
("ldk-server", "payments") so existing protobuf-encoded data is
silently ignored rather than failing to deserialize, avoiding the
need for migration code or manual database wipes.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Remove the RabbitMQ-based EventPublisher trait and lapin dependency
in favor of a built-in SSE streaming endpoint. Events are now
delivered directly to clients over HTTP/TLS via a /Subscribe
endpoint, eliminating the need for an external message broker.
The server uses a tokio broadcast channel internally. The new
SseBody type implements hyper::body::Body to stream events as
JSON in SSE format. Event publishing becomes synchronous and
fire-and-forget when no subscribers are connected.
Add a subscribe command to the CLI that connects to the SSE
endpoint and prints each event as a JSON line to stdout. The
client library exposes a typed async event stream via
LdkServerClient::subscribe().
E2E tests use CliEventConsumer which spawns the CLI subscribe
command as a child process, replacing the previous raw TLS/SSE
consumer and RabbitMQ consumer.
AI tools were used in preparing this commit.
Add CORS headers to all server responses and handle OPTIONS
preflight requests, enabling browser-based clients to connect
directly to ldk-server.
Include a single-file web UI (index.html) that demonstrates
connecting to the JSON API and SSE event stream from the browser
using @microsoft/fetch-event-source. The UI shows node info,
lists peers with keysend buttons, supports BOLT11 payments, and
displays the live event stream.
AI tools were used in preparing this commit.
@ldk-reviews-bot

Copy link
Copy Markdown

👋 Hi! I see this is a draft PR.
I'll wait to assign reviewers until you mark it as ready for review.
Just convert it out of draft status when you're ready for review!

Full::new(Bytes::new()).boxed()
}

fn with_cors_headers(mut response: ServiceResponse) -> ServiceResponse {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

we probably want this configurable

@benthecarmanbenthecarman left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

this is great overall!

[dependencies]
ldk-server-json-models = { path = "../ldk-server-json-models" }
reqwest = { version = "0.11.13", default-features = false, features = ["rustls-tls"] }
reqwest = { version = "0.11.13", default-features = false, features = ["rustls-tls", "stream"] }

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I know we are planning on moving to bitreq, is this something that it supports?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I don't believe we do, no.

let forwarded_payment_creation_time = SystemTime::now().duration_since(UNIX_EPOCH).expect("Time must be > 1970").as_secs() as i64;

match event_publisher.publish(
let _ = event_sender.send(

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

we should still handle the error properly

}
}

fn error_to_response(e: LdkServerError) -> ServiceResponse {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Wouldn't this be better as just a From impl? Then we can just do ?

pub const GRAPH_GET_CHANNEL_PATH: &str = "GraphGetChannel";
pub const GRAPH_LIST_NODES_PATH: &str = "GraphListNodes";
pub const GRAPH_GET_NODE_PATH: &str = "GraphGetNode";
pub const SUBSCRIBE_PATH: &str = "Subscribe";

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Curious on your guy's thoughts on adding something like Subscribe/Payment/<id> and then you just get the events for that id.

Could see this nice for being able to permission a client to only certain events. Also makes it so you can just have a single stream for a payment instead of needing to filter between all events.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Might be useful indeed. I would only add it after user demand though

Comment threade2e-tests/src/lib.rs
use lapin::types::FieldTable;
use lapin::{ConnectionProperties, ExchangeKind};
impl CliEventConsumer {
/// Start the CLI subscribe command and begin receiving events in the background.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

using the cli here seems a little overly complex but i guess thats more e2e so maybe that's correct.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

I was in doubt about that too. Saw cli was already used, so I thought, make it as e2e as possible...

Comment on lines +459 to +468
let payload = response.bytes().await.map_err(|e| {
LdkServerError::new(InternalError, format!("Failed to read response body: {}", e))
})?;
let error_response =
serde_json::from_slice::<ErrorResponse>(&payload).map_err(|e| {
LdkServerError::new(
JsonParseError,
format!("Failed to decode error response (status {}): {}", status, e),
)
})?;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

You should be able to do let error_response: ErrorResponse = response.json() to clean this up a bunch

Ok(c) => c,
Err(_) => break,
};
buffer.push_str(&String::from_utf8_lossy(&chunk));

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Is there any potential concern here with multiple byte characters being split between multiple chunks and causing errors here?

Err(_) => break,
};
buffer.push_str(&String::from_utf8_lossy(&chunk));
while let Some(pos) = buffer.find("\n\n") {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I know this is handling for the SSE format but would be good to have some comments here explaining

> {
use futures_util::StreamExt;
let url = format!("https://{}/{SUBSCRIBE_PATH}", self.base_url);
let auth_header = self.compute_auth_header(&[]);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

unrelated to this PR but realizing the auth header should probably commit to the endpoint too

Derive ToSchema on all request/response and domain types in
ldk-server-json-models, with #[schema(value_type = String)] overrides
for hex-serialized fields. Define all 36 API path operations in a new
openapi module using utoipa's path macro on stub functions, grouped
by tags (Node, Onchain, Bolt11, Bolt12, Channels, Payments, Peers,
Send, Graph, Crypto, Events). The spec is served at /openapi.json
without authentication, cached via LazyLock.
AI tools were used in preparing this commit.
Sign up for freeto 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

@joostjager@ldk-reviews-bot@tnull@benthecarman
, 'i'); if (__m === '*' || __re.test(location.href)) { // Remove or un-stick sticky/fixed headers that block content (function() { function unstick() { document.querySelectorAll('header, nav, [role="banner"], .header, .navbar, .sticky, .fixed-top, [style*="position: fixed"], [style*="position:sticky"]').forEach(function(el) { if (el.style.position === 'fixed' || el.style.position === 'sticky' || getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') { el.style.position = 'static'; el.style.top = 'auto'; el.style.zIndex = 'auto'; } }); } unstick(); var observer = new MutationObserver(unstick); observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] }); })(); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' Add server-side events and html UI [PoC] by joostjager · Pull Request #171 · lightningdevkit/ldk-server · GitHub
Skip to content

Add server-side events and html UI [PoC] - #171

Draft
joostjager wants to merge 4 commits into
lightningdevkit:mainfrom
joostjager:sse
Draft

Add server-side events and html UI [PoC]#171
joostjager wants to merge 4 commits into
lightningdevkit:mainfrom
joostjager:sse

Conversation

@joostjager

Copy link
Copy Markdown
Contributor
image

Based on #168

benthecarmanand others added 3 commits March 23, 2026 19:50
Protobuf added complexity without much benefit for our use case — the
binary encoding is opaque, hard to debug with standard HTTP tools, and
requires proto toolchain maintenance. JSON is human-readable, widely
supported, and sufficient for our throughput needs.
This removes prost and all .proto files entirely, renaming the
ldk-server-protos crate to ldk-server-json-models. Types are rewritten
as hand-written Rust structs and enums with serde derives rather than
prost-generated code. Fixed-size byte fields (hashes, channel IDs,
public keys) use [u8; 32] and [u8; 33] with hex serde instead of
String, giving type safety at the model layer.
Several proto-era patterns are cleaned up: wrapper structs that only
existed because protobuf wraps oneof in a message are removed, fields
that were Option only because proto message fields are nullable are
made required where the server always provides them, and the
EventEnvelope wrapper is dropped in favor of using Event directly.
Storage namespaces are changed from ("payments", "") to
("ldk-server", "payments") so existing protobuf-encoded data is
silently ignored rather than failing to deserialize, avoiding the
need for migration code or manual database wipes.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Remove the RabbitMQ-based EventPublisher trait and lapin dependency
in favor of a built-in SSE streaming endpoint. Events are now
delivered directly to clients over HTTP/TLS via a /Subscribe
endpoint, eliminating the need for an external message broker.
The server uses a tokio broadcast channel internally. The new
SseBody type implements hyper::body::Body to stream events as
JSON in SSE format. Event publishing becomes synchronous and
fire-and-forget when no subscribers are connected.
Add a subscribe command to the CLI that connects to the SSE
endpoint and prints each event as a JSON line to stdout. The
client library exposes a typed async event stream via
LdkServerClient::subscribe().
E2E tests use CliEventConsumer which spawns the CLI subscribe
command as a child process, replacing the previous raw TLS/SSE
consumer and RabbitMQ consumer.
AI tools were used in preparing this commit.
Add CORS headers to all server responses and handle OPTIONS
preflight requests, enabling browser-based clients to connect
directly to ldk-server.
Include a single-file web UI (index.html) that demonstrates
connecting to the JSON API and SSE event stream from the browser
using @microsoft/fetch-event-source. The UI shows node info,
lists peers with keysend buttons, supports BOLT11 payments, and
displays the live event stream.
AI tools were used in preparing this commit.
@ldk-reviews-bot

Copy link
Copy Markdown

👋 Hi! I see this is a draft PR.
I'll wait to assign reviewers until you mark it as ready for review.
Just convert it out of draft status when you're ready for review!

Full::new(Bytes::new()).boxed()
}

fn with_cors_headers(mut response: ServiceResponse) -> ServiceResponse {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

we probably want this configurable

@benthecarmanbenthecarman left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

this is great overall!

[dependencies]
ldk-server-json-models = { path = "../ldk-server-json-models" }
reqwest = { version = "0.11.13", default-features = false, features = ["rustls-tls"] }
reqwest = { version = "0.11.13", default-features = false, features = ["rustls-tls", "stream"] }

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I know we are planning on moving to bitreq, is this something that it supports?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I don't believe we do, no.

let forwarded_payment_creation_time = SystemTime::now().duration_since(UNIX_EPOCH).expect("Time must be > 1970").as_secs() as i64;

match event_publisher.publish(
let _ = event_sender.send(

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

we should still handle the error properly

}
}

fn error_to_response(e: LdkServerError) -> ServiceResponse {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Wouldn't this be better as just a From impl? Then we can just do ?

pub const GRAPH_GET_CHANNEL_PATH: &str = "GraphGetChannel";
pub const GRAPH_LIST_NODES_PATH: &str = "GraphListNodes";
pub const GRAPH_GET_NODE_PATH: &str = "GraphGetNode";
pub const SUBSCRIBE_PATH: &str = "Subscribe";

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Curious on your guy's thoughts on adding something like Subscribe/Payment/<id> and then you just get the events for that id.

Could see this nice for being able to permission a client to only certain events. Also makes it so you can just have a single stream for a payment instead of needing to filter between all events.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Might be useful indeed. I would only add it after user demand though

Comment threade2e-tests/src/lib.rs
use lapin::types::FieldTable;
use lapin::{ConnectionProperties, ExchangeKind};
impl CliEventConsumer {
/// Start the CLI subscribe command and begin receiving events in the background.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

using the cli here seems a little overly complex but i guess thats more e2e so maybe that's correct.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

I was in doubt about that too. Saw cli was already used, so I thought, make it as e2e as possible...

Comment on lines +459 to +468
let payload = response.bytes().await.map_err(|e| {
LdkServerError::new(InternalError, format!("Failed to read response body: {}", e))
})?;
let error_response =
serde_json::from_slice::<ErrorResponse>(&payload).map_err(|e| {
LdkServerError::new(
JsonParseError,
format!("Failed to decode error response (status {}): {}", status, e),
)
})?;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

You should be able to do let error_response: ErrorResponse = response.json() to clean this up a bunch

Ok(c) => c,
Err(_) => break,
};
buffer.push_str(&String::from_utf8_lossy(&chunk));

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Is there any potential concern here with multiple byte characters being split between multiple chunks and causing errors here?

Err(_) => break,
};
buffer.push_str(&String::from_utf8_lossy(&chunk));
while let Some(pos) = buffer.find("\n\n") {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I know this is handling for the SSE format but would be good to have some comments here explaining

> {
use futures_util::StreamExt;
let url = format!("https://{}/{SUBSCRIBE_PATH}", self.base_url);
let auth_header = self.compute_auth_header(&[]);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

unrelated to this PR but realizing the auth header should probably commit to the endpoint too

Derive ToSchema on all request/response and domain types in
ldk-server-json-models, with #[schema(value_type = String)] overrides
for hex-serialized fields. Define all 36 API path operations in a new
openapi module using utoipa's path macro on stub functions, grouped
by tags (Node, Onchain, Bolt11, Bolt12, Channels, Payments, Peers,
Send, Graph, Crypto, Events). The spec is served at /openapi.json
without authentication, cached via LazyLock.
AI tools were used in preparing this commit.
Sign up for freeto 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

@joostjager@ldk-reviews-bot@tnull@benthecarman
, 'i'); if (__m === '*' || __re.test(location.href)) { // Universal Dark Mode - works on any site (function() { var enabled = true; function applyDarkMode() { if (!enabled) return; // Create style element if it doesn't exist var style = document.getElementById('universal-dark-mode-style'); if (!style) { style = document.createElement('style'); style.id = 'universal-dark-mode-style'; document.head.appendChild(style); } // Dark mode CSS - inverts colors but preserves images/video style.textContent = ' /* Invert everything except media */ html { filter: invert(1) hue-rotate(180deg) !important; background: #1a1a2e !important; } /* Restore images, videos, iframes, canvas */ img, video, iframe, canvas, svg, picture, [style*="background-image"] { filter: invert(1) hue-rotate(180deg) !important; } /* Preserve specific elements that should not be inverted */ .no-dark-mode, .no-dark-mode *, [data-theme="light"], [data-theme="light"], .ace_editor, .ace_editor *, .CodeMirror, .CodeMirror *, .monaco-editor, .monaco-editor *, .markdown-body pre, .markdown-body pre *, .highlight, .highlight *, pre code, pre code * { filter: none !important; } /* Fix common UI elements */ .modal, .popup, .dropdown-menu, .tooltip, .popover { filter: invert(1) hue-rotate(180deg) !important; background: #2d2d44 !important; border-color: #444 !important; } /* Scrollbars */ ::-webkit-scrollbar { background: #1a1a2e !important; } ::-webkit-scrollbar-thumb { background: #444 !important; } ::-webkit-scrollbar-thumb:hover { background: #555 !important; } /* Selection */ ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; } ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; } '; } function removeDarkMode() { var style = document.getElementById('universal-dark-mode-style'); if (style) style.remove(); } // Toggle with Alt+Shift+D document.addEventListener('keydown', function(e) { if (e.altKey && e.shiftKey && e.key === 'D') { e.preventDefault(); enabled = !enabled; if (enabled) { applyDarkMode(); console.log('[Universal Dark Mode] Enabled'); } else { removeDarkMode(); console.log('[Universal Dark Mode] Disabled'); } } }); // Apply on load applyDarkMode(); // Re-apply on dynamic content var observer = new MutationObserver(function(mutations) { if (enabled && !document.getElementById('universal-dark-mode-style')) { applyDarkMode(); } }); observer.observe(document.head, { childList: true }); console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle'); })(); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })(); Add server-side events and html UI [PoC] by joostjager · Pull Request #171 · lightningdevkit/ldk-server · GitHub
Skip to content

Add server-side events and html UI [PoC] - #171

Draft
joostjager wants to merge 4 commits into
lightningdevkit:mainfrom
joostjager:sse
Draft

Add server-side events and html UI [PoC]#171
joostjager wants to merge 4 commits into
lightningdevkit:mainfrom
joostjager:sse

Conversation

@joostjager

Copy link
Copy Markdown
Contributor
image

Based on #168

benthecarmanand others added 3 commits March 23, 2026 19:50
Protobuf added complexity without much benefit for our use case — the
binary encoding is opaque, hard to debug with standard HTTP tools, and
requires proto toolchain maintenance. JSON is human-readable, widely
supported, and sufficient for our throughput needs.
This removes prost and all .proto files entirely, renaming the
ldk-server-protos crate to ldk-server-json-models. Types are rewritten
as hand-written Rust structs and enums with serde derives rather than
prost-generated code. Fixed-size byte fields (hashes, channel IDs,
public keys) use [u8; 32] and [u8; 33] with hex serde instead of
String, giving type safety at the model layer.
Several proto-era patterns are cleaned up: wrapper structs that only
existed because protobuf wraps oneof in a message are removed, fields
that were Option only because proto message fields are nullable are
made required where the server always provides them, and the
EventEnvelope wrapper is dropped in favor of using Event directly.
Storage namespaces are changed from ("payments", "") to
("ldk-server", "payments") so existing protobuf-encoded data is
silently ignored rather than failing to deserialize, avoiding the
need for migration code or manual database wipes.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Remove the RabbitMQ-based EventPublisher trait and lapin dependency
in favor of a built-in SSE streaming endpoint. Events are now
delivered directly to clients over HTTP/TLS via a /Subscribe
endpoint, eliminating the need for an external message broker.
The server uses a tokio broadcast channel internally. The new
SseBody type implements hyper::body::Body to stream events as
JSON in SSE format. Event publishing becomes synchronous and
fire-and-forget when no subscribers are connected.
Add a subscribe command to the CLI that connects to the SSE
endpoint and prints each event as a JSON line to stdout. The
client library exposes a typed async event stream via
LdkServerClient::subscribe().
E2E tests use CliEventConsumer which spawns the CLI subscribe
command as a child process, replacing the previous raw TLS/SSE
consumer and RabbitMQ consumer.
AI tools were used in preparing this commit.
Add CORS headers to all server responses and handle OPTIONS
preflight requests, enabling browser-based clients to connect
directly to ldk-server.
Include a single-file web UI (index.html) that demonstrates
connecting to the JSON API and SSE event stream from the browser
using @microsoft/fetch-event-source. The UI shows node info,
lists peers with keysend buttons, supports BOLT11 payments, and
displays the live event stream.
AI tools were used in preparing this commit.
@ldk-reviews-bot

Copy link
Copy Markdown

👋 Hi! I see this is a draft PR.
I'll wait to assign reviewers until you mark it as ready for review.
Just convert it out of draft status when you're ready for review!

Full::new(Bytes::new()).boxed()
}

fn with_cors_headers(mut response: ServiceResponse) -> ServiceResponse {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

we probably want this configurable

@benthecarmanbenthecarman left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

this is great overall!

[dependencies]
ldk-server-json-models = { path = "../ldk-server-json-models" }
reqwest = { version = "0.11.13", default-features = false, features = ["rustls-tls"] }
reqwest = { version = "0.11.13", default-features = false, features = ["rustls-tls", "stream"] }

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I know we are planning on moving to bitreq, is this something that it supports?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I don't believe we do, no.

let forwarded_payment_creation_time = SystemTime::now().duration_since(UNIX_EPOCH).expect("Time must be > 1970").as_secs() as i64;

match event_publisher.publish(
let _ = event_sender.send(

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

we should still handle the error properly

}
}

fn error_to_response(e: LdkServerError) -> ServiceResponse {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Wouldn't this be better as just a From impl? Then we can just do ?

pub const GRAPH_GET_CHANNEL_PATH: &str = "GraphGetChannel";
pub const GRAPH_LIST_NODES_PATH: &str = "GraphListNodes";
pub const GRAPH_GET_NODE_PATH: &str = "GraphGetNode";
pub const SUBSCRIBE_PATH: &str = "Subscribe";

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Curious on your guy's thoughts on adding something like Subscribe/Payment/<id> and then you just get the events for that id.

Could see this nice for being able to permission a client to only certain events. Also makes it so you can just have a single stream for a payment instead of needing to filter between all events.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Might be useful indeed. I would only add it after user demand though

Comment threade2e-tests/src/lib.rs
use lapin::types::FieldTable;
use lapin::{ConnectionProperties, ExchangeKind};
impl CliEventConsumer {
/// Start the CLI subscribe command and begin receiving events in the background.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

using the cli here seems a little overly complex but i guess thats more e2e so maybe that's correct.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

I was in doubt about that too. Saw cli was already used, so I thought, make it as e2e as possible...

Comment on lines +459 to +468
let payload = response.bytes().await.map_err(|e| {
LdkServerError::new(InternalError, format!("Failed to read response body: {}", e))
})?;
let error_response =
serde_json::from_slice::<ErrorResponse>(&payload).map_err(|e| {
LdkServerError::new(
JsonParseError,
format!("Failed to decode error response (status {}): {}", status, e),
)
})?;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

You should be able to do let error_response: ErrorResponse = response.json() to clean this up a bunch

Ok(c) => c,
Err(_) => break,
};
buffer.push_str(&String::from_utf8_lossy(&chunk));

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Is there any potential concern here with multiple byte characters being split between multiple chunks and causing errors here?

Err(_) => break,
};
buffer.push_str(&String::from_utf8_lossy(&chunk));
while let Some(pos) = buffer.find("\n\n") {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I know this is handling for the SSE format but would be good to have some comments here explaining

> {
use futures_util::StreamExt;
let url = format!("https://{}/{SUBSCRIBE_PATH}", self.base_url);
let auth_header = self.compute_auth_header(&[]);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

unrelated to this PR but realizing the auth header should probably commit to the endpoint too

Derive ToSchema on all request/response and domain types in
ldk-server-json-models, with #[schema(value_type = String)] overrides
for hex-serialized fields. Define all 36 API path operations in a new
openapi module using utoipa's path macro on stub functions, grouped
by tags (Node, Onchain, Bolt11, Bolt12, Channels, Payments, Peers,
Send, Graph, Crypto, Events). The spec is served at /openapi.json
without authentication, cached via LazyLock.
AI tools were used in preparing this commit.
Sign up for freeto 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

@joostjager@ldk-reviews-bot@tnull@benthecarman