Skip to content

Implement first set of Api's #5

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 9 commits into from
Sep 25, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

13 changes: 3 additions & 10 deletions protos/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -87,15 +87,8 @@ pub struct Bolt11SendRequest {
#[derive(Clone, PartialEq, ::prost::Message)]
pub struct Bolt11SendResponse {
/// An identifier used to uniquely identify a payment.
#[prost(message, optional, tag = "1")]
pub payment_id: ::core::option::Option<PaymentId>,
}
/// An identifier used to uniquely identify a payment.
#[allow(clippy::derive_partial_eq_without_eq)]
#[derive(Clone, PartialEq, ::prost::Message)]
pub struct PaymentId {
#[prost(bytes = "bytes", tag = "1")]
pub data: ::prost::bytes::Bytes,
pub payment_id: ::prost::bytes::Bytes,
}
/// Returns a BOLT12 offer for the given amount, if specified.
///
Expand Down Expand Up @@ -148,8 +141,8 @@ pub struct Bolt12SendRequest {
#[derive(Clone, PartialEq, ::prost::Message)]
pub struct Bolt12SendResponse {
/// An identifier used to uniquely identify a payment.
#[prost(message, optional, tag = "1")]
pub payment_id: ::core::option::Option<PaymentId>,
#[prost(bytes = "bytes", tag = "1")]
pub payment_id: ::prost::bytes::Bytes,
}
/// Creates a new outbound channel to the given remote node.
/// See more: <https://docs.rs/ldk-node/latest/ldk_node/struct.Node.html#method.connect_open_channel>
Expand Down
10 changes: 2 additions & 8 deletions protos/src/proto/ldk_node_server.proto
Original file line number Diff line number Diff line change
Expand Up @@ -85,13 +85,7 @@ message Bolt11SendRequest {
message Bolt11SendResponse {

// An identifier used to uniquely identify a payment.
PaymentId payment_id = 1;
}

// An identifier used to uniquely identify a payment.
message PaymentId {

bytes data = 1;
bytes payment_id = 1;
}

// Returns a BOLT12 offer for the given amount, if specified.
Expand Down Expand Up @@ -141,7 +135,7 @@ message Bolt12SendRequest {
message Bolt12SendResponse {

// An identifier used to uniquely identify a payment.
PaymentId payment_id = 1;
bytes payment_id = 1;
}

// Creates a new outbound channel to the given remote node.
Expand Down
1 change: 1 addition & 0 deletions server/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -13,3 +13,4 @@ hyper-util = { version = "0.1", default-features = false, features = ["server-gr
tokio = { version = "1.38.0", default-features = false, features = ["time", "signal", "rt-multi-thread"] }
prost = { version = "0.11.6", default-features = false, features = ["std"] }
protos = { path = "../protos" }
bytes = "1.4.0"
21 changes: 21 additions & 0 deletions server/src/api/bolt11_receive.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
use ldk_node::Node;
use protos::{Bolt11ReceiveRequest, Bolt11ReceiveResponse};
use std::sync::Arc;

pub(crate) const BOLT11_RECEIVE_PATH: &str = "Bolt11Receive";

pub(crate) fn handle_bolt11_receive_request(
node: Arc<Node>, request: Bolt11ReceiveRequest,
) -> Result<Bolt11ReceiveResponse, ldk_node::NodeError> {
let invoice = match request.amount_msat {
Some(amount_msat) => {
node.bolt11_payment().receive(amount_msat, &request.description, request.expiry_secs)?
},
None => node
.bolt11_payment()
.receive_variable_amount(&request.description, request.expiry_secs)?,
};

let response = Bolt11ReceiveResponse { invoice: invoice.to_string() };
Ok(response)
}
23 changes: 23 additions & 0 deletions server/src/api/bolt11_send.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
use bytes::Bytes;
use ldk_node::lightning_invoice::Bolt11Invoice;
use ldk_node::Node;
use protos::{Bolt11SendRequest, Bolt11SendResponse};
use std::str::FromStr;
use std::sync::Arc;

pub(crate) const BOLT11_SEND_PATH: &str = "Bolt11Send";

pub(crate) fn handle_bolt11_send_request(
node: Arc<Node>, request: Bolt11SendRequest,
) -> Result<Bolt11SendResponse, ldk_node::NodeError> {
let invoice = Bolt11Invoice::from_str(&request.invoice.as_str())
.map_err(|_| ldk_node::NodeError::InvalidInvoice)?;

let payment_id = match request.amount_msat {
None => node.bolt11_payment().send(&invoice),
Some(amount_msat) => node.bolt11_payment().send_using_amount(&invoice, amount_msat),
}?;

let response = Bolt11SendResponse { payment_id: Bytes::from(payment_id.0.to_vec()) };
Ok(response)
}
17 changes: 17 additions & 0 deletions server/src/api/bolt12_receive.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
use ldk_node::Node;
use protos::{Bolt12ReceiveRequest, Bolt12ReceiveResponse};
use std::sync::Arc;

pub(crate) const BOLT12_RECEIVE_PATH: &str = "Bolt12Receive";

pub(crate) fn handle_bolt12_receive_request(
node: Arc<Node>, request: Bolt12ReceiveRequest,
) -> Result<Bolt12ReceiveResponse, ldk_node::NodeError> {
let offer = match request.amount_msat {
Some(amount_msat) => node.bolt12_payment().receive(amount_msat, &request.description)?,
None => node.bolt12_payment().receive_variable_amount(&request.description)?,
};

let response = Bolt12ReceiveResponse { offer: offer.to_string() };
Ok(response)
}
25 changes: 25 additions & 0 deletions server/src/api/bolt12_send.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
use bytes::Bytes;
use ldk_node::lightning::offers::offer::Offer;
use ldk_node::Node;
use protos::{Bolt12SendRequest, Bolt12SendResponse};
use std::str::FromStr;
use std::sync::Arc;

pub(crate) const BOLT12_SEND_PATH: &str = "Bolt12Send";

pub(crate) fn handle_bolt12_send_request(
node: Arc<Node>, request: Bolt12SendRequest,
) -> Result<Bolt12SendResponse, ldk_node::NodeError> {
let offer =
Offer::from_str(&request.offer.as_str()).map_err(|_| ldk_node::NodeError::InvalidOffer)?;

let payment_id = match request.amount_msat {
None => node.bolt12_payment().send(&offer, request.payer_note),
Some(amount_msat) => {
node.bolt12_payment().send_using_amount(&offer, request.payer_note, amount_msat)
},
}?;

let response = Bolt12SendResponse { payment_id: Bytes::from(payment_id.0.to_vec()) };
Ok(response)
}
26 changes: 26 additions & 0 deletions server/src/api/close_channel.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
use ldk_node::bitcoin::secp256k1::PublicKey;
use ldk_node::{Node, UserChannelId};
use protos::{CloseChannelRequest, CloseChannelResponse};
use std::str::FromStr;
use std::sync::Arc;

pub(crate) const CLOSE_CHANNEL_PATH: &str = "CloseChannel";

pub(crate) fn handle_close_channel_request(
node: Arc<Node>, request: CloseChannelRequest,
) -> Result<CloseChannelResponse, ldk_node::NodeError> {
//TODO: Should this be string?
Copy link
Collaborator

Choose a reason for hiding this comment

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

IMO, since PaymentId is its dedicated proto type, UserChannelId/ChannelId, etc. should probably also follow the same pattern?

Copy link
Contributor Author

@G8XSU G8XSU Sep 11, 2024

Choose a reason for hiding this comment

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

@tnull @jkczyz
The main problem is user interaction with these byte identifiers, for which I could use some ideas. Essentially, IDs such as ChannelId, UserChannelId, and PaymentId are not human-readable in byte form and cannot be easily input or output as bytes.

Using bytes is fine for programmatic access, but for CLI or while interacting with a lightning node through a UI, this isn't really feasible. We have a similar problem for the logs of these IDs as well. (See: lightningdevkit/rust-lightning#3306)

In my opinion, we need a standardized way of interacting with these in a human-friendly manner, not just for debug logs. For this, I was considering whether we can use a hex representation string in the interface. This could be just for the CLI/UI or as part of the main API interface.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Mhh, I don't have a strong opinion, but I do think it should be uniform, at least across all *Id types.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I thought more about this,
I don't think adding separate types for *Id helps from API perspective. It just introduces further nesting of types.
It might make sense in ldk-node api to introduce special types where you can have type checking but not in an api.

{
payment_id :PaymentId{data:[..]},
user_channel_id: UserChannelId{data:[..]},
channel_id: ChannelId{data:[..]},
}
compared to:  
{
payment_id :[..],
user_channel_id: [..],
channel_id: [..],
}

Bolt11SendResponse { payment_id: Some(PaymentId { data: payment_id.0.to_vec() }) }
compared to 
Bolt11SendResponse { payment_id: payment_id.0.to_vec() }

So to make it uniform, I am thinking of removing PaymentId type itself.

Copy link
Collaborator

@tnull tnull Sep 25, 2024

Choose a reason for hiding this comment

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

So to make it uniform, I am thinking of removing PaymentId type itself.

Alright, fine by me as long as it's uniform and ~predictable by the user/dev so they don't have to look up every single detail in the docs when using the API. And, at this stage, we could still easily change anything if we find a reason why we need separate types in the future.

Feel free to add the commit dropping PaymentId here before we land this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes added a commit for it.
Squashed and rebased.

let mut user_channel_id_bytes = [0u8; 16];
user_channel_id_bytes.copy_from_slice(&request.user_channel_id);
let user_channel_id = UserChannelId(u128::from_be_bytes(user_channel_id_bytes));
let counterparty_node_id = PublicKey::from_str(&request.counterparty_node_id)
.map_err(|_| ldk_node::NodeError::InvalidPublicKey)?;

match request.force_close {
Some(true) => node.force_close_channel(&user_channel_id, counterparty_node_id)?,
_ => node.close_channel(&user_channel_id, counterparty_node_id)?,
};

let response = CloseChannelResponse {};
Ok(response)
}
8 changes: 8 additions & 0 deletions server/src/api/mod.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
pub(crate) mod bolt11_receive;
pub(crate) mod bolt11_send;
pub(crate) mod bolt12_receive;
pub(crate) mod bolt12_send;
pub(crate) mod close_channel;
pub(crate) mod onchain_receive;
pub(crate) mod onchain_send;
pub(crate) mod open_channel;
12 changes: 12 additions & 0 deletions server/src/api/onchain_receive.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
use ldk_node::Node;
use protos::{OnchainReceiveRequest, OnchainReceiveResponse};
use std::sync::Arc;

pub(crate) const ONCHAIN_RECEIVE_PATH: &str = "OnchainReceive";
Copy link
Collaborator

Choose a reason for hiding this comment

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

Mh, fine by me that we dropped the actual /path/structure/, but given that the path is still part of a URL, should we still make these lower case and possibly even dash-case, i.e., onchain-receive?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Wanted to avoid another mapping from api to api path, hence used it as it is. (i.e. /OnchainReceive for OnchainReceiveRequest and OnchainReceiveResponse.)
Since it is internal only, I don't think it matters much.

pub(crate) fn handle_onchain_receive_request(
node: Arc<Node>, _request: OnchainReceiveRequest,
) -> Result<OnchainReceiveResponse, ldk_node::NodeError> {
let response =
OnchainReceiveResponse { address: node.onchain_payment().new_address()?.to_string() };
Ok(response)
}
25 changes: 25 additions & 0 deletions server/src/api/onchain_send.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
use ldk_node::bitcoin::Address;
use ldk_node::Node;
use protos::{OnchainSendRequest, OnchainSendResponse};
use std::str::FromStr;
use std::sync::Arc;

pub(crate) const ONCHAIN_SEND_PATH: &str = "OnchainSend";

pub(crate) fn handle_onchain_send_request(
node: Arc<Node>, request: OnchainSendRequest,
) -> Result<OnchainSendResponse, ldk_node::NodeError> {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Might be fine for the intial step, but we might need to introduce a separate error type that wraps the NodeError, i.e., will be a superset?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah currently it is only for initial step, will probably spend more time on error handling when we have it in api interface.

let address = Address::from_str(&request.address)
.map_err(|_| ldk_node::NodeError::InvalidAddress)?
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should we import NodeError to avoid the prefix?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I kept it to make it explicit when there are other error types floating around, specifically ldk-server specific.
But I dont mind it either way.

.require_network(node.config().network)
Copy link
Collaborator

Choose a reason for hiding this comment

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

I wonder if it would be worth retrieving the Config once and then giving a &Config to the handler methods rather than always calling in? Would at least avoid cloning it every time.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not all api's need it, and there could be future such instances where a subset of api's need some field.
I think we shouldn't add it to common handler for now.

.map_err(|_| ldk_node::NodeError::InvalidAddress)?;
let txid = match (request.amount_sats, request.send_all) {
(Some(amount_sats), None) => {
node.onchain_payment().send_to_address(&address, amount_sats)?
},
(None, Some(true)) => node.onchain_payment().send_all_to_address(&address)?,
_ => return Err(ldk_node::NodeError::InvalidAmount),
};
let response = OnchainSendResponse { txid: txid.to_string() };
Ok(response)
}
31 changes: 31 additions & 0 deletions server/src/api/open_channel.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
use bytes::Bytes;
use ldk_node::bitcoin::secp256k1::PublicKey;
use ldk_node::lightning::ln::msgs::SocketAddress;
use ldk_node::Node;
use protos::{OpenChannelRequest, OpenChannelResponse};
use std::str::FromStr;
use std::sync::Arc;

pub(crate) const OPEN_CHANNEL_PATH: &str = "OpenChannel";

pub(crate) fn handle_open_channel(
node: Arc<Node>, request: OpenChannelRequest,
) -> Result<OpenChannelResponse, ldk_node::NodeError> {
let node_id = PublicKey::from_str(&request.node_pubkey)
.map_err(|_| ldk_node::NodeError::InvalidPublicKey)?;
let address = SocketAddress::from_str(&request.address)
.map_err(|_| ldk_node::NodeError::InvalidSocketAddress)?;
let user_channel_id = node.connect_open_channel(
node_id,
address,
request.channel_amount_sats,
request.push_to_counterparty_msat,
// TODO: Allow setting ChannelConfig in open-channel.
None,
request.announce_channel,
)?;
let response = OpenChannelResponse {
user_channel_id: Bytes::from(user_channel_id.0.to_be_bytes().to_vec()),
};
Ok(response)
}
1 change: 1 addition & 0 deletions server/src/main.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
mod api;
mod service;

use crate::service::NodeService;
Expand Down
33 changes: 32 additions & 1 deletion server/src/service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,23 @@ use std::future::Future;
use std::pin::Pin;
use std::sync::Arc;

use crate::api::bolt11_receive::handle_bolt11_receive_request;
use crate::api::bolt11_receive::BOLT11_RECEIVE_PATH;
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: Could group the imports from each module, but doesn't matter too much.

use crate::api::bolt11_send::handle_bolt11_send_request;
use crate::api::bolt11_send::BOLT11_SEND_PATH;
use crate::api::bolt12_receive::handle_bolt12_receive_request;
use crate::api::bolt12_receive::BOLT12_RECEIVE_PATH;
use crate::api::bolt12_send::handle_bolt12_send_request;
use crate::api::bolt12_send::BOLT12_SEND_PATH;
use crate::api::close_channel::handle_close_channel_request;
use crate::api::close_channel::CLOSE_CHANNEL_PATH;
use crate::api::onchain_receive::handle_onchain_receive_request;
use crate::api::onchain_receive::ONCHAIN_RECEIVE_PATH;
use crate::api::onchain_send::handle_onchain_send_request;
use crate::api::onchain_send::ONCHAIN_SEND_PATH;
use crate::api::open_channel::handle_open_channel;
use crate::api::open_channel::OPEN_CHANNEL_PATH;

#[derive(Clone)]
pub struct NodeService {
node: Arc<Node>,
Expand All @@ -28,8 +45,22 @@ impl Service<Request<Incoming>> for NodeService {
type Future = Pin<Box<dyn Future<Output = Result<Self::Response, Self::Error>> + Send>>;

fn call(&self, req: Request<Incoming>) -> Self::Future {
let _node = Arc::clone(&self.node);
let node = Arc::clone(&self.node);
match req.uri().path() {
ONCHAIN_RECEIVE_PATH => {
Box::pin(handle_request(node, req, handle_onchain_receive_request))
},
ONCHAIN_SEND_PATH => Box::pin(handle_request(node, req, handle_onchain_send_request)),
BOLT11_RECEIVE_PATH => {
Box::pin(handle_request(node, req, handle_bolt11_receive_request))
},
BOLT11_SEND_PATH => Box::pin(handle_request(node, req, handle_bolt11_send_request)),
BOLT12_RECEIVE_PATH => {
Box::pin(handle_request(node, req, handle_bolt12_receive_request))
},
BOLT12_SEND_PATH => Box::pin(handle_request(node, req, handle_bolt12_send_request)),
OPEN_CHANNEL_PATH => Box::pin(handle_request(node, req, handle_open_channel)),
CLOSE_CHANNEL_PATH => Box::pin(handle_request(node, req, handle_close_channel_request)),
path => {
let error = format!("Unknown request: {}", path).into_bytes();
Box::pin(async {
Expand Down