Skip to content

Conversation

@benthecarman
Copy link
Collaborator

This basically fills out the remaining ldk-node functions that were not used by the RPC/CLI.

@ldk-reviews-bot
Copy link

ldk-reviews-bot commented Jan 22, 2026

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

@joostjager
Copy link
Contributor

joostjager commented Jan 22, 2026

One general question that I had for ldk-server is whether we want some light e2e testing? cli->client->server path.

Perhaps an LdkNode trait is a possibility to mock ldk-node and focus testing on ldk-server.

@benthecarman
Copy link
Collaborator Author

One general question that I had for ldk-server is whether we want some light e2e testing? cli->client->server path.

Yeah that would be nice, probably better for a separate PR though

@joostjager
Copy link
Contributor

Yes, agreed on the separate PR for this. It's pre-existing.

joostjager
joostjager previously approved these changes Jan 23, 2026
Copy link
Contributor

@joostjager joostjager left a comment

Choose a reason for hiding this comment

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

Untested ack

LdkServerError::new(InvalidRequestError, "Invalid node_id provided.".to_string())
})?;

let route_parameters = match request.route_parameters {
Copy link
Contributor

Choose a reason for hiding this comment

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

Possible to reuse handle_bolt11_send_request matching code?

@tnull
Copy link
Collaborator

tnull commented Jan 23, 2026

One general question that I had for ldk-server is whether we want some light e2e testing? cli->client->server path.

Perhaps an LdkNode trait is a possibility to mock ldk-node and focus testing on ldk-server.

Relatedly, I recently opened lightningdevkit/ldk-node#766 and tagged it as a BOSS topic. We could however also consider whether we want to move the CLN/LND/Eclair interop tests to the server repo, rather than doing them at the LDK Node level? Although, it is somewhat nice to do it like the other integration tests in Rust.

tnull
tnull previously approved these changes Jan 23, 2026
Copy link
Collaborator

@tnull tnull left a comment

Choose a reason for hiding this comment

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

ACK, opened #114 as a follow-up.

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

Labels

None yet

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

4 participants