Add base REST Client and CLI setup. by G8XSU · Pull Request #6 · lightningdevkit/ldk-server

G8XSU · 2024-09-06T22:07:22Z

Add base REST client setup.
Add base cli setup.
Add initial set of api methods to rest-client.

jkczyz · 2024-09-11T16:51:22Z

+	async fn post_request<Rq: Message, Rs: Message + Default>(
+ &self, request: &Rq, url: &str,
+	) -> Result<Rs, LdkNodeServerError> {
+ let request_body = request.encode_to_vec();


Instead of allocating a Vec, should we instead use something like async_prost (https://docs.rs/async-prost/latest/async_prost/) or prost_stream (https://docs.rs/prost-stream/latest/prost_stream/) with Body::wrap_stream (https://docs.rs/reqwest/latest/reqwest/struct.Body.html#method.wrap_stream)? Not sure how reliable those are or if we want to take on the dependencies, though I suppose we could implement TryStream ourselves.

Both of libs aren't maintained at all, prost is maintained by tokio.
Imo, we should stick with the popular and well-used lib for now and piggyback on them improving performance for having future optimizations like this.
Since both encode and decode are internal details, we can change them at any time without any breakage, so i am not too worried about this optimization for now.
Also, currently our messages have limited size and don't really contain any big blobs, so its upside is limited.
However it did motivate me to look into some other optimization which is to replace all vec with Bytes, which is easily doable and could avoid some vec clones at some of the places.
While in decode, prost has special handling for ref-counted byte buffers, it will not clone them, will return a ref to slice instead. Reqwest already returns ref-counted Bytes as response.
Also, I am not sure how much of improvement it is if any, we can send reqwest a Bytes object instead of vec and it can create body from that. In the current version of reqwest that we are using, stream trait for body isn't stable iiuc.
It mentions "// The Stream trait isn't stable, so the impl isn't public.".

Both of libs aren't maintained at all, prost is maintained by tokio. Imo, we should stick with the popular and well-used lib for now and piggyback on them improving performance for having future optimizations like this. Since both encode and decode are internal details, we can change them at any time without any breakage, so i am not too worried about this optimization for now. Also, currently our messages have limited size and don't really contain any big blobs, so its upside is limited.
Fair enough! Thanks for investigating though.
However it did motivate me to look into some other optimization which is to replace all vec with Bytes, which is easily doable and could avoid some vec clones at some of the places.
Which clones are avoided? Or are you saying in another PR?
While in decode, prost has special handling for ref-counted byte buffers, it will not clone them, will return a ref to slice instead. Reqwest already returns ref-counted Bytes as response.
Not sure how this is relevant. Aren't we talking about encoding objects that we've created in memory?
Also, I am not sure how much of improvement it is if any, we can send reqwest a Bytes object instead of vec and it can create body from that.
Isn't that just a wrapper around a Vec in how we are using it? Nothing needs to be shared, IIUC.
In the current version of reqwest that we are using, stream trait for body isn't stable iiuc.
It mentions "// The Stream trait isn't stable, so the impl isn't public.".
Hmm... I see it as an optional feature not unstable.

Which clones are avoided? Or are you saying in another PR?
Yes, mostly while creating these proto objects if we already have access to a byte slice or buffer.
Not sure how this is relevant. Aren't we talking about encoding objects that we've created in memory?
Reqwest returns Bytes, it will read raw bytes from wire and place them on buffer. if we change generated protos to use Bytes, it can refer to a slice from that buffer, whereas if we use vec it has to clone that part of memory and allocate while converting from slice iiuc.
Isn't that just a wrapper around a Vec in how we are using it? Nothing needs to be shared, IIUC.
Yeah this doesn't change much of anything. It is mostly some minor optimizations that bytes might do internally like buffer allocation/reuse or optimizing storage of small Bytes objects.
Hmm... I see it as an optional feature not unstable.
It was mentioned in code: https://github.com/seanmonstar/reqwest/blame/cf69fd4bfe22855d576497eb94e9eb549e742475/src/async_impl/body.rs#L22
Iiuc, support for streaming as public api was only exposed last week: seanmonstar/reqwest#2255

jkczyz · 2024-09-11T23:36:04Z

+	async fn post_request<Rq: Message, Rs: Message + Default>(
+ &self, request: &Rq, url: &str,
+	) -> Result<Rs, LdkNodeServerError> {
+ let request_body = request.encode_to_vec();


Both of libs aren't maintained at all, prost is maintained by tokio. Imo, we should stick with the popular and well-used lib for now and piggyback on them improving performance for having future optimizations like this. Since both encode and decode are internal details, we can change them at any time without any breakage, so i am not too worried about this optimization for now. Also, currently our messages have limited size and don't really contain any big blobs, so its upside is limited.
Fair enough! Thanks for investigating though.
However it did motivate me to look into some other optimization which is to replace all vec with Bytes, which is easily doable and could avoid some vec clones at some of the places.
Which clones are avoided? Or are you saying in another PR?
While in decode, prost has special handling for ref-counted byte buffers, it will not clone them, will return a ref to slice instead. Reqwest already returns ref-counted Bytes as response.
Not sure how this is relevant. Aren't we talking about encoding objects that we've created in memory?
Also, I am not sure how much of improvement it is if any, we can send reqwest a Bytes object instead of vec and it can create body from that.
Isn't that just a wrapper around a Vec in how we are using it? Nothing needs to be shared, IIUC.
In the current version of reqwest that we are using, stream trait for body isn't stable iiuc.
It mentions "// The Stream trait isn't stable, so the impl isn't public.".
Hmm... I see it as an optional feature not unstable.

jkczyz

LGTM. Please squash.

G8XSU · 2024-09-12T21:14:07Z

Rebased on main and squashed.

G8XSU · 2024-09-06T22:07:22Z

Add base REST client setup.
Add base cli setup.
Add initial set of api methods to rest-client.

jkczyz · 2024-09-11T16:51:22Z

+	async fn post_request<Rq: Message, Rs: Message + Default>(
+ &self, request: &Rq, url: &str,
+	) -> Result<Rs, LdkNodeServerError> {
+ let request_body = request.encode_to_vec();


Instead of allocating a Vec, should we instead use something like async_prost (https://docs.rs/async-prost/latest/async_prost/) or prost_stream (https://docs.rs/prost-stream/latest/prost_stream/) with Body::wrap_stream (https://docs.rs/reqwest/latest/reqwest/struct.Body.html#method.wrap_stream)? Not sure how reliable those are or if we want to take on the dependencies, though I suppose we could implement TryStream ourselves.

Both of libs aren't maintained at all, prost is maintained by tokio.
Imo, we should stick with the popular and well-used lib for now and piggyback on them improving performance for having future optimizations like this.
Since both encode and decode are internal details, we can change them at any time without any breakage, so i am not too worried about this optimization for now.
Also, currently our messages have limited size and don't really contain any big blobs, so its upside is limited.
However it did motivate me to look into some other optimization which is to replace all vec with Bytes, which is easily doable and could avoid some vec clones at some of the places.
While in decode, prost has special handling for ref-counted byte buffers, it will not clone them, will return a ref to slice instead. Reqwest already returns ref-counted Bytes as response.
Also, I am not sure how much of improvement it is if any, we can send reqwest a Bytes object instead of vec and it can create body from that. In the current version of reqwest that we are using, stream trait for body isn't stable iiuc.
It mentions "// The Stream trait isn't stable, so the impl isn't public.".

Both of libs aren't maintained at all, prost is maintained by tokio. Imo, we should stick with the popular and well-used lib for now and piggyback on them improving performance for having future optimizations like this. Since both encode and decode are internal details, we can change them at any time without any breakage, so i am not too worried about this optimization for now. Also, currently our messages have limited size and don't really contain any big blobs, so its upside is limited.
Fair enough! Thanks for investigating though.
However it did motivate me to look into some other optimization which is to replace all vec with Bytes, which is easily doable and could avoid some vec clones at some of the places.
Which clones are avoided? Or are you saying in another PR?
While in decode, prost has special handling for ref-counted byte buffers, it will not clone them, will return a ref to slice instead. Reqwest already returns ref-counted Bytes as response.
Not sure how this is relevant. Aren't we talking about encoding objects that we've created in memory?
Also, I am not sure how much of improvement it is if any, we can send reqwest a Bytes object instead of vec and it can create body from that.
Isn't that just a wrapper around a Vec in how we are using it? Nothing needs to be shared, IIUC.
In the current version of reqwest that we are using, stream trait for body isn't stable iiuc.
It mentions "// The Stream trait isn't stable, so the impl isn't public.".
Hmm... I see it as an optional feature not unstable.

Which clones are avoided? Or are you saying in another PR?
Yes, mostly while creating these proto objects if we already have access to a byte slice or buffer.
Not sure how this is relevant. Aren't we talking about encoding objects that we've created in memory?
Reqwest returns Bytes, it will read raw bytes from wire and place them on buffer. if we change generated protos to use Bytes, it can refer to a slice from that buffer, whereas if we use vec it has to clone that part of memory and allocate while converting from slice iiuc.
Isn't that just a wrapper around a Vec in how we are using it? Nothing needs to be shared, IIUC.
Yeah this doesn't change much of anything. It is mostly some minor optimizations that bytes might do internally like buffer allocation/reuse or optimizing storage of small Bytes objects.
Hmm... I see it as an optional feature not unstable.
It was mentioned in code: https://github.com/seanmonstar/reqwest/blame/cf69fd4bfe22855d576497eb94e9eb549e742475/src/async_impl/body.rs#L22
Iiuc, support for streaming as public api was only exposed last week: seanmonstar/reqwest#2255

jkczyz · 2024-09-11T23:36:04Z

+	async fn post_request<Rq: Message, Rs: Message + Default>(
+ &self, request: &Rq, url: &str,
+	) -> Result<Rs, LdkNodeServerError> {
+ let request_body = request.encode_to_vec();


Both of libs aren't maintained at all, prost is maintained by tokio. Imo, we should stick with the popular and well-used lib for now and piggyback on them improving performance for having future optimizations like this. Since both encode and decode are internal details, we can change them at any time without any breakage, so i am not too worried about this optimization for now. Also, currently our messages have limited size and don't really contain any big blobs, so its upside is limited.
Fair enough! Thanks for investigating though.
However it did motivate me to look into some other optimization which is to replace all vec with Bytes, which is easily doable and could avoid some vec clones at some of the places.
Which clones are avoided? Or are you saying in another PR?
While in decode, prost has special handling for ref-counted byte buffers, it will not clone them, will return a ref to slice instead. Reqwest already returns ref-counted Bytes as response.
Not sure how this is relevant. Aren't we talking about encoding objects that we've created in memory?
Also, I am not sure how much of improvement it is if any, we can send reqwest a Bytes object instead of vec and it can create body from that.
Isn't that just a wrapper around a Vec in how we are using it? Nothing needs to be shared, IIUC.
In the current version of reqwest that we are using, stream trait for body isn't stable iiuc.
It mentions "// The Stream trait isn't stable, so the impl isn't public.".
Hmm... I see it as an optional feature not unstable.

jkczyz

LGTM. Please squash.

G8XSU · 2024-09-12T21:14:07Z

Rebased on main and squashed.

G8XSU · 2024-09-06T22:07:22Z

Add base REST client setup.
Add base cli setup.
Add initial set of api methods to rest-client.

jkczyz · 2024-09-11T16:51:22Z

+	async fn post_request<Rq: Message, Rs: Message + Default>(
+ &self, request: &Rq, url: &str,
+	) -> Result<Rs, LdkNodeServerError> {
+ let request_body = request.encode_to_vec();


Instead of allocating a Vec, should we instead use something like async_prost (https://docs.rs/async-prost/latest/async_prost/) or prost_stream (https://docs.rs/prost-stream/latest/prost_stream/) with Body::wrap_stream (https://docs.rs/reqwest/latest/reqwest/struct.Body.html#method.wrap_stream)? Not sure how reliable those are or if we want to take on the dependencies, though I suppose we could implement TryStream ourselves.

Both of libs aren't maintained at all, prost is maintained by tokio.
Imo, we should stick with the popular and well-used lib for now and piggyback on them improving performance for having future optimizations like this.
Since both encode and decode are internal details, we can change them at any time without any breakage, so i am not too worried about this optimization for now.
Also, currently our messages have limited size and don't really contain any big blobs, so its upside is limited.
However it did motivate me to look into some other optimization which is to replace all vec with Bytes, which is easily doable and could avoid some vec clones at some of the places.
While in decode, prost has special handling for ref-counted byte buffers, it will not clone them, will return a ref to slice instead. Reqwest already returns ref-counted Bytes as response.
Also, I am not sure how much of improvement it is if any, we can send reqwest a Bytes object instead of vec and it can create body from that. In the current version of reqwest that we are using, stream trait for body isn't stable iiuc.
It mentions "// The Stream trait isn't stable, so the impl isn't public.".

Both of libs aren't maintained at all, prost is maintained by tokio. Imo, we should stick with the popular and well-used lib for now and piggyback on them improving performance for having future optimizations like this. Since both encode and decode are internal details, we can change them at any time without any breakage, so i am not too worried about this optimization for now. Also, currently our messages have limited size and don't really contain any big blobs, so its upside is limited.
Fair enough! Thanks for investigating though.
However it did motivate me to look into some other optimization which is to replace all vec with Bytes, which is easily doable and could avoid some vec clones at some of the places.
Which clones are avoided? Or are you saying in another PR?
While in decode, prost has special handling for ref-counted byte buffers, it will not clone them, will return a ref to slice instead. Reqwest already returns ref-counted Bytes as response.
Not sure how this is relevant. Aren't we talking about encoding objects that we've created in memory?
Also, I am not sure how much of improvement it is if any, we can send reqwest a Bytes object instead of vec and it can create body from that.
Isn't that just a wrapper around a Vec in how we are using it? Nothing needs to be shared, IIUC.
In the current version of reqwest that we are using, stream trait for body isn't stable iiuc.
It mentions "// The Stream trait isn't stable, so the impl isn't public.".
Hmm... I see it as an optional feature not unstable.

Which clones are avoided? Or are you saying in another PR?
Yes, mostly while creating these proto objects if we already have access to a byte slice or buffer.
Not sure how this is relevant. Aren't we talking about encoding objects that we've created in memory?
Reqwest returns Bytes, it will read raw bytes from wire and place them on buffer. if we change generated protos to use Bytes, it can refer to a slice from that buffer, whereas if we use vec it has to clone that part of memory and allocate while converting from slice iiuc.
Isn't that just a wrapper around a Vec in how we are using it? Nothing needs to be shared, IIUC.
Yeah this doesn't change much of anything. It is mostly some minor optimizations that bytes might do internally like buffer allocation/reuse or optimizing storage of small Bytes objects.
Hmm... I see it as an optional feature not unstable.
It was mentioned in code: https://github.com/seanmonstar/reqwest/blame/cf69fd4bfe22855d576497eb94e9eb549e742475/src/async_impl/body.rs#L22
Iiuc, support for streaming as public api was only exposed last week: seanmonstar/reqwest#2255

jkczyz · 2024-09-11T23:36:04Z

+	async fn post_request<Rq: Message, Rs: Message + Default>(
+ &self, request: &Rq, url: &str,
+	) -> Result<Rs, LdkNodeServerError> {
+ let request_body = request.encode_to_vec();


Both of libs aren't maintained at all, prost is maintained by tokio. Imo, we should stick with the popular and well-used lib for now and piggyback on them improving performance for having future optimizations like this. Since both encode and decode are internal details, we can change them at any time without any breakage, so i am not too worried about this optimization for now. Also, currently our messages have limited size and don't really contain any big blobs, so its upside is limited.
Fair enough! Thanks for investigating though.
However it did motivate me to look into some other optimization which is to replace all vec with Bytes, which is easily doable and could avoid some vec clones at some of the places.
Which clones are avoided? Or are you saying in another PR?
While in decode, prost has special handling for ref-counted byte buffers, it will not clone them, will return a ref to slice instead. Reqwest already returns ref-counted Bytes as response.
Not sure how this is relevant. Aren't we talking about encoding objects that we've created in memory?
Also, I am not sure how much of improvement it is if any, we can send reqwest a Bytes object instead of vec and it can create body from that.
Isn't that just a wrapper around a Vec in how we are using it? Nothing needs to be shared, IIUC.
In the current version of reqwest that we are using, stream trait for body isn't stable iiuc.
It mentions "// The Stream trait isn't stable, so the impl isn't public.".
Hmm... I see it as an optional feature not unstable.

jkczyz

LGTM. Please squash.

G8XSU · 2024-09-12T21:14:07Z

Rebased on main and squashed.

G8XSU · 2024-09-06T22:07:22Z

Add base REST client setup.
Add base cli setup.
Add initial set of api methods to rest-client.

jkczyz · 2024-09-11T16:51:22Z

+	async fn post_request<Rq: Message, Rs: Message + Default>(
+ &self, request: &Rq, url: &str,
+	) -> Result<Rs, LdkNodeServerError> {
+ let request_body = request.encode_to_vec();


Instead of allocating a Vec, should we instead use something like async_prost (https://docs.rs/async-prost/latest/async_prost/) or prost_stream (https://docs.rs/prost-stream/latest/prost_stream/) with Body::wrap_stream (https://docs.rs/reqwest/latest/reqwest/struct.Body.html#method.wrap_stream)? Not sure how reliable those are or if we want to take on the dependencies, though I suppose we could implement TryStream ourselves.

Both of libs aren't maintained at all, prost is maintained by tokio.
Imo, we should stick with the popular and well-used lib for now and piggyback on them improving performance for having future optimizations like this.
Since both encode and decode are internal details, we can change them at any time without any breakage, so i am not too worried about this optimization for now.
Also, currently our messages have limited size and don't really contain any big blobs, so its upside is limited.
However it did motivate me to look into some other optimization which is to replace all vec with Bytes, which is easily doable and could avoid some vec clones at some of the places.
While in decode, prost has special handling for ref-counted byte buffers, it will not clone them, will return a ref to slice instead. Reqwest already returns ref-counted Bytes as response.
Also, I am not sure how much of improvement it is if any, we can send reqwest a Bytes object instead of vec and it can create body from that. In the current version of reqwest that we are using, stream trait for body isn't stable iiuc.
It mentions "// The Stream trait isn't stable, so the impl isn't public.".

Both of libs aren't maintained at all, prost is maintained by tokio. Imo, we should stick with the popular and well-used lib for now and piggyback on them improving performance for having future optimizations like this. Since both encode and decode are internal details, we can change them at any time without any breakage, so i am not too worried about this optimization for now. Also, currently our messages have limited size and don't really contain any big blobs, so its upside is limited.
Fair enough! Thanks for investigating though.
However it did motivate me to look into some other optimization which is to replace all vec with Bytes, which is easily doable and could avoid some vec clones at some of the places.
Which clones are avoided? Or are you saying in another PR?
While in decode, prost has special handling for ref-counted byte buffers, it will not clone them, will return a ref to slice instead. Reqwest already returns ref-counted Bytes as response.
Not sure how this is relevant. Aren't we talking about encoding objects that we've created in memory?
Also, I am not sure how much of improvement it is if any, we can send reqwest a Bytes object instead of vec and it can create body from that.
Isn't that just a wrapper around a Vec in how we are using it? Nothing needs to be shared, IIUC.
In the current version of reqwest that we are using, stream trait for body isn't stable iiuc.
It mentions "// The Stream trait isn't stable, so the impl isn't public.".
Hmm... I see it as an optional feature not unstable.

Which clones are avoided? Or are you saying in another PR?
Yes, mostly while creating these proto objects if we already have access to a byte slice or buffer.
Not sure how this is relevant. Aren't we talking about encoding objects that we've created in memory?
Reqwest returns Bytes, it will read raw bytes from wire and place them on buffer. if we change generated protos to use Bytes, it can refer to a slice from that buffer, whereas if we use vec it has to clone that part of memory and allocate while converting from slice iiuc.
Isn't that just a wrapper around a Vec in how we are using it? Nothing needs to be shared, IIUC.
Yeah this doesn't change much of anything. It is mostly some minor optimizations that bytes might do internally like buffer allocation/reuse or optimizing storage of small Bytes objects.
Hmm... I see it as an optional feature not unstable.
It was mentioned in code: https://github.com/seanmonstar/reqwest/blame/cf69fd4bfe22855d576497eb94e9eb549e742475/src/async_impl/body.rs#L22
Iiuc, support for streaming as public api was only exposed last week: seanmonstar/reqwest#2255

jkczyz · 2024-09-11T23:36:04Z

+	async fn post_request<Rq: Message, Rs: Message + Default>(
+ &self, request: &Rq, url: &str,
+	) -> Result<Rs, LdkNodeServerError> {
+ let request_body = request.encode_to_vec();


Both of libs aren't maintained at all, prost is maintained by tokio. Imo, we should stick with the popular and well-used lib for now and piggyback on them improving performance for having future optimizations like this. Since both encode and decode are internal details, we can change them at any time without any breakage, so i am not too worried about this optimization for now. Also, currently our messages have limited size and don't really contain any big blobs, so its upside is limited.
Fair enough! Thanks for investigating though.
However it did motivate me to look into some other optimization which is to replace all vec with Bytes, which is easily doable and could avoid some vec clones at some of the places.
Which clones are avoided? Or are you saying in another PR?
While in decode, prost has special handling for ref-counted byte buffers, it will not clone them, will return a ref to slice instead. Reqwest already returns ref-counted Bytes as response.
Not sure how this is relevant. Aren't we talking about encoding objects that we've created in memory?
Also, I am not sure how much of improvement it is if any, we can send reqwest a Bytes object instead of vec and it can create body from that.
Isn't that just a wrapper around a Vec in how we are using it? Nothing needs to be shared, IIUC.
In the current version of reqwest that we are using, stream trait for body isn't stable iiuc.
It mentions "// The Stream trait isn't stable, so the impl isn't public.".
Hmm... I see it as an optional feature not unstable.

jkczyz

LGTM. Please squash.

G8XSU · 2024-09-12T21:14:07Z

Rebased on main and squashed.

G8XSU · 2024-09-06T22:07:22Z

Add base REST client setup.
Add base cli setup.
Add initial set of api methods to rest-client.

jkczyz · 2024-09-11T16:51:22Z

+	async fn post_request<Rq: Message, Rs: Message + Default>(
+ &self, request: &Rq, url: &str,
+	) -> Result<Rs, LdkNodeServerError> {
+ let request_body = request.encode_to_vec();


Instead of allocating a Vec, should we instead use something like async_prost (https://docs.rs/async-prost/latest/async_prost/) or prost_stream (https://docs.rs/prost-stream/latest/prost_stream/) with Body::wrap_stream (https://docs.rs/reqwest/latest/reqwest/struct.Body.html#method.wrap_stream)? Not sure how reliable those are or if we want to take on the dependencies, though I suppose we could implement TryStream ourselves.

Both of libs aren't maintained at all, prost is maintained by tokio.
Imo, we should stick with the popular and well-used lib for now and piggyback on them improving performance for having future optimizations like this.
Since both encode and decode are internal details, we can change them at any time without any breakage, so i am not too worried about this optimization for now.
Also, currently our messages have limited size and don't really contain any big blobs, so its upside is limited.
However it did motivate me to look into some other optimization which is to replace all vec with Bytes, which is easily doable and could avoid some vec clones at some of the places.
While in decode, prost has special handling for ref-counted byte buffers, it will not clone them, will return a ref to slice instead. Reqwest already returns ref-counted Bytes as response.
Also, I am not sure how much of improvement it is if any, we can send reqwest a Bytes object instead of vec and it can create body from that. In the current version of reqwest that we are using, stream trait for body isn't stable iiuc.
It mentions "// The Stream trait isn't stable, so the impl isn't public.".

Both of libs aren't maintained at all, prost is maintained by tokio. Imo, we should stick with the popular and well-used lib for now and piggyback on them improving performance for having future optimizations like this. Since both encode and decode are internal details, we can change them at any time without any breakage, so i am not too worried about this optimization for now. Also, currently our messages have limited size and don't really contain any big blobs, so its upside is limited.
Fair enough! Thanks for investigating though.
However it did motivate me to look into some other optimization which is to replace all vec with Bytes, which is easily doable and could avoid some vec clones at some of the places.
Which clones are avoided? Or are you saying in another PR?
While in decode, prost has special handling for ref-counted byte buffers, it will not clone them, will return a ref to slice instead. Reqwest already returns ref-counted Bytes as response.
Not sure how this is relevant. Aren't we talking about encoding objects that we've created in memory?
Also, I am not sure how much of improvement it is if any, we can send reqwest a Bytes object instead of vec and it can create body from that.
Isn't that just a wrapper around a Vec in how we are using it? Nothing needs to be shared, IIUC.
In the current version of reqwest that we are using, stream trait for body isn't stable iiuc.
It mentions "// The Stream trait isn't stable, so the impl isn't public.".
Hmm... I see it as an optional feature not unstable.

Which clones are avoided? Or are you saying in another PR?
Yes, mostly while creating these proto objects if we already have access to a byte slice or buffer.
Not sure how this is relevant. Aren't we talking about encoding objects that we've created in memory?
Reqwest returns Bytes, it will read raw bytes from wire and place them on buffer. if we change generated protos to use Bytes, it can refer to a slice from that buffer, whereas if we use vec it has to clone that part of memory and allocate while converting from slice iiuc.
Isn't that just a wrapper around a Vec in how we are using it? Nothing needs to be shared, IIUC.
Yeah this doesn't change much of anything. It is mostly some minor optimizations that bytes might do internally like buffer allocation/reuse or optimizing storage of small Bytes objects.
Hmm... I see it as an optional feature not unstable.
It was mentioned in code: https://github.com/seanmonstar/reqwest/blame/cf69fd4bfe22855d576497eb94e9eb549e742475/src/async_impl/body.rs#L22
Iiuc, support for streaming as public api was only exposed last week: seanmonstar/reqwest#2255

jkczyz · 2024-09-11T23:36:04Z

+	async fn post_request<Rq: Message, Rs: Message + Default>(
+ &self, request: &Rq, url: &str,
+	) -> Result<Rs, LdkNodeServerError> {
+ let request_body = request.encode_to_vec();


Both of libs aren't maintained at all, prost is maintained by tokio. Imo, we should stick with the popular and well-used lib for now and piggyback on them improving performance for having future optimizations like this. Since both encode and decode are internal details, we can change them at any time without any breakage, so i am not too worried about this optimization for now. Also, currently our messages have limited size and don't really contain any big blobs, so its upside is limited.
Fair enough! Thanks for investigating though.
However it did motivate me to look into some other optimization which is to replace all vec with Bytes, which is easily doable and could avoid some vec clones at some of the places.
Which clones are avoided? Or are you saying in another PR?
While in decode, prost has special handling for ref-counted byte buffers, it will not clone them, will return a ref to slice instead. Reqwest already returns ref-counted Bytes as response.
Not sure how this is relevant. Aren't we talking about encoding objects that we've created in memory?
Also, I am not sure how much of improvement it is if any, we can send reqwest a Bytes object instead of vec and it can create body from that.
Isn't that just a wrapper around a Vec in how we are using it? Nothing needs to be shared, IIUC.
In the current version of reqwest that we are using, stream trait for body isn't stable iiuc.
It mentions "// The Stream trait isn't stable, so the impl isn't public.".
Hmm... I see it as an optional feature not unstable.

jkczyz

LGTM. Please squash.

G8XSU · 2024-09-12T21:14:07Z

Rebased on main and squashed.

G8XSU · 2024-09-06T22:07:22Z

Add base REST client setup.
Add base cli setup.
Add initial set of api methods to rest-client.

jkczyz · 2024-09-11T16:51:22Z

+	async fn post_request<Rq: Message, Rs: Message + Default>(
+ &self, request: &Rq, url: &str,
+	) -> Result<Rs, LdkNodeServerError> {
+ let request_body = request.encode_to_vec();


Instead of allocating a Vec, should we instead use something like async_prost (https://docs.rs/async-prost/latest/async_prost/) or prost_stream (https://docs.rs/prost-stream/latest/prost_stream/) with Body::wrap_stream (https://docs.rs/reqwest/latest/reqwest/struct.Body.html#method.wrap_stream)? Not sure how reliable those are or if we want to take on the dependencies, though I suppose we could implement TryStream ourselves.

Both of libs aren't maintained at all, prost is maintained by tokio.
Imo, we should stick with the popular and well-used lib for now and piggyback on them improving performance for having future optimizations like this.
Since both encode and decode are internal details, we can change them at any time without any breakage, so i am not too worried about this optimization for now.
Also, currently our messages have limited size and don't really contain any big blobs, so its upside is limited.
However it did motivate me to look into some other optimization which is to replace all vec with Bytes, which is easily doable and could avoid some vec clones at some of the places.
While in decode, prost has special handling for ref-counted byte buffers, it will not clone them, will return a ref to slice instead. Reqwest already returns ref-counted Bytes as response.
Also, I am not sure how much of improvement it is if any, we can send reqwest a Bytes object instead of vec and it can create body from that. In the current version of reqwest that we are using, stream trait for body isn't stable iiuc.
It mentions "// The Stream trait isn't stable, so the impl isn't public.".

Both of libs aren't maintained at all, prost is maintained by tokio. Imo, we should stick with the popular and well-used lib for now and piggyback on them improving performance for having future optimizations like this. Since both encode and decode are internal details, we can change them at any time without any breakage, so i am not too worried about this optimization for now. Also, currently our messages have limited size and don't really contain any big blobs, so its upside is limited.
Fair enough! Thanks for investigating though.
However it did motivate me to look into some other optimization which is to replace all vec with Bytes, which is easily doable and could avoid some vec clones at some of the places.
Which clones are avoided? Or are you saying in another PR?
While in decode, prost has special handling for ref-counted byte buffers, it will not clone them, will return a ref to slice instead. Reqwest already returns ref-counted Bytes as response.
Not sure how this is relevant. Aren't we talking about encoding objects that we've created in memory?
Also, I am not sure how much of improvement it is if any, we can send reqwest a Bytes object instead of vec and it can create body from that.
Isn't that just a wrapper around a Vec in how we are using it? Nothing needs to be shared, IIUC.
In the current version of reqwest that we are using, stream trait for body isn't stable iiuc.
It mentions "// The Stream trait isn't stable, so the impl isn't public.".
Hmm... I see it as an optional feature not unstable.

Which clones are avoided? Or are you saying in another PR?
Yes, mostly while creating these proto objects if we already have access to a byte slice or buffer.
Not sure how this is relevant. Aren't we talking about encoding objects that we've created in memory?
Reqwest returns Bytes, it will read raw bytes from wire and place them on buffer. if we change generated protos to use Bytes, it can refer to a slice from that buffer, whereas if we use vec it has to clone that part of memory and allocate while converting from slice iiuc.
Isn't that just a wrapper around a Vec in how we are using it? Nothing needs to be shared, IIUC.
Yeah this doesn't change much of anything. It is mostly some minor optimizations that bytes might do internally like buffer allocation/reuse or optimizing storage of small Bytes objects.
Hmm... I see it as an optional feature not unstable.
It was mentioned in code: https://github.com/seanmonstar/reqwest/blame/cf69fd4bfe22855d576497eb94e9eb549e742475/src/async_impl/body.rs#L22
Iiuc, support for streaming as public api was only exposed last week: seanmonstar/reqwest#2255

jkczyz · 2024-09-11T23:36:04Z

+	async fn post_request<Rq: Message, Rs: Message + Default>(
+ &self, request: &Rq, url: &str,
+	) -> Result<Rs, LdkNodeServerError> {
+ let request_body = request.encode_to_vec();


Both of libs aren't maintained at all, prost is maintained by tokio. Imo, we should stick with the popular and well-used lib for now and piggyback on them improving performance for having future optimizations like this. Since both encode and decode are internal details, we can change them at any time without any breakage, so i am not too worried about this optimization for now. Also, currently our messages have limited size and don't really contain any big blobs, so its upside is limited.
Fair enough! Thanks for investigating though.
However it did motivate me to look into some other optimization which is to replace all vec with Bytes, which is easily doable and could avoid some vec clones at some of the places.
Which clones are avoided? Or are you saying in another PR?
While in decode, prost has special handling for ref-counted byte buffers, it will not clone them, will return a ref to slice instead. Reqwest already returns ref-counted Bytes as response.
Not sure how this is relevant. Aren't we talking about encoding objects that we've created in memory?
Also, I am not sure how much of improvement it is if any, we can send reqwest a Bytes object instead of vec and it can create body from that.
Isn't that just a wrapper around a Vec in how we are using it? Nothing needs to be shared, IIUC.
In the current version of reqwest that we are using, stream trait for body isn't stable iiuc.
It mentions "// The Stream trait isn't stable, so the impl isn't public.".
Hmm... I see it as an optional feature not unstable.

jkczyz

LGTM. Please squash.

G8XSU · 2024-09-12T21:14:07Z

Rebased on main and squashed.

G8XSU · 2024-09-06T22:07:22Z

Add base REST client setup.
Add base cli setup.
Add initial set of api methods to rest-client.

jkczyz · 2024-09-11T16:51:22Z

+	async fn post_request<Rq: Message, Rs: Message + Default>(
+ &self, request: &Rq, url: &str,
+	) -> Result<Rs, LdkNodeServerError> {
+ let request_body = request.encode_to_vec();


Instead of allocating a Vec, should we instead use something like async_prost (https://docs.rs/async-prost/latest/async_prost/) or prost_stream (https://docs.rs/prost-stream/latest/prost_stream/) with Body::wrap_stream (https://docs.rs/reqwest/latest/reqwest/struct.Body.html#method.wrap_stream)? Not sure how reliable those are or if we want to take on the dependencies, though I suppose we could implement TryStream ourselves.

Both of libs aren't maintained at all, prost is maintained by tokio.
Imo, we should stick with the popular and well-used lib for now and piggyback on them improving performance for having future optimizations like this.
Since both encode and decode are internal details, we can change them at any time without any breakage, so i am not too worried about this optimization for now.
Also, currently our messages have limited size and don't really contain any big blobs, so its upside is limited.
However it did motivate me to look into some other optimization which is to replace all vec with Bytes, which is easily doable and could avoid some vec clones at some of the places.
While in decode, prost has special handling for ref-counted byte buffers, it will not clone them, will return a ref to slice instead. Reqwest already returns ref-counted Bytes as response.
Also, I am not sure how much of improvement it is if any, we can send reqwest a Bytes object instead of vec and it can create body from that. In the current version of reqwest that we are using, stream trait for body isn't stable iiuc.
It mentions "// The Stream trait isn't stable, so the impl isn't public.".

Both of libs aren't maintained at all, prost is maintained by tokio. Imo, we should stick with the popular and well-used lib for now and piggyback on them improving performance for having future optimizations like this. Since both encode and decode are internal details, we can change them at any time without any breakage, so i am not too worried about this optimization for now. Also, currently our messages have limited size and don't really contain any big blobs, so its upside is limited.
Fair enough! Thanks for investigating though.
However it did motivate me to look into some other optimization which is to replace all vec with Bytes, which is easily doable and could avoid some vec clones at some of the places.
Which clones are avoided? Or are you saying in another PR?
While in decode, prost has special handling for ref-counted byte buffers, it will not clone them, will return a ref to slice instead. Reqwest already returns ref-counted Bytes as response.
Not sure how this is relevant. Aren't we talking about encoding objects that we've created in memory?
Also, I am not sure how much of improvement it is if any, we can send reqwest a Bytes object instead of vec and it can create body from that.
Isn't that just a wrapper around a Vec in how we are using it? Nothing needs to be shared, IIUC.
In the current version of reqwest that we are using, stream trait for body isn't stable iiuc.
It mentions "// The Stream trait isn't stable, so the impl isn't public.".
Hmm... I see it as an optional feature not unstable.

Which clones are avoided? Or are you saying in another PR?
Yes, mostly while creating these proto objects if we already have access to a byte slice or buffer.
Not sure how this is relevant. Aren't we talking about encoding objects that we've created in memory?
Reqwest returns Bytes, it will read raw bytes from wire and place them on buffer. if we change generated protos to use Bytes, it can refer to a slice from that buffer, whereas if we use vec it has to clone that part of memory and allocate while converting from slice iiuc.
Isn't that just a wrapper around a Vec in how we are using it? Nothing needs to be shared, IIUC.
Yeah this doesn't change much of anything. It is mostly some minor optimizations that bytes might do internally like buffer allocation/reuse or optimizing storage of small Bytes objects.
Hmm... I see it as an optional feature not unstable.
It was mentioned in code: https://github.com/seanmonstar/reqwest/blame/cf69fd4bfe22855d576497eb94e9eb549e742475/src/async_impl/body.rs#L22
Iiuc, support for streaming as public api was only exposed last week: seanmonstar/reqwest#2255

jkczyz · 2024-09-11T23:36:04Z

+	async fn post_request<Rq: Message, Rs: Message + Default>(
+ &self, request: &Rq, url: &str,
+	) -> Result<Rs, LdkNodeServerError> {
+ let request_body = request.encode_to_vec();


Both of libs aren't maintained at all, prost is maintained by tokio. Imo, we should stick with the popular and well-used lib for now and piggyback on them improving performance for having future optimizations like this. Since both encode and decode are internal details, we can change them at any time without any breakage, so i am not too worried about this optimization for now. Also, currently our messages have limited size and don't really contain any big blobs, so its upside is limited.
Fair enough! Thanks for investigating though.
However it did motivate me to look into some other optimization which is to replace all vec with Bytes, which is easily doable and could avoid some vec clones at some of the places.
Which clones are avoided? Or are you saying in another PR?
While in decode, prost has special handling for ref-counted byte buffers, it will not clone them, will return a ref to slice instead. Reqwest already returns ref-counted Bytes as response.
Not sure how this is relevant. Aren't we talking about encoding objects that we've created in memory?
Also, I am not sure how much of improvement it is if any, we can send reqwest a Bytes object instead of vec and it can create body from that.
Isn't that just a wrapper around a Vec in how we are using it? Nothing needs to be shared, IIUC.
In the current version of reqwest that we are using, stream trait for body isn't stable iiuc.
It mentions "// The Stream trait isn't stable, so the impl isn't public.".
Hmm... I see it as an optional feature not unstable.

jkczyz

LGTM. Please squash.

G8XSU · 2024-09-12T21:14:07Z

Rebased on main and squashed.

G8XSU · 2024-09-06T22:07:22Z

Add base REST client setup.
Add base cli setup.
Add initial set of api methods to rest-client.

jkczyz · 2024-09-11T16:51:22Z

+	async fn post_request<Rq: Message, Rs: Message + Default>(
+ &self, request: &Rq, url: &str,
+	) -> Result<Rs, LdkNodeServerError> {
+ let request_body = request.encode_to_vec();


Instead of allocating a Vec, should we instead use something like async_prost (https://docs.rs/async-prost/latest/async_prost/) or prost_stream (https://docs.rs/prost-stream/latest/prost_stream/) with Body::wrap_stream (https://docs.rs/reqwest/latest/reqwest/struct.Body.html#method.wrap_stream)? Not sure how reliable those are or if we want to take on the dependencies, though I suppose we could implement TryStream ourselves.

Both of libs aren't maintained at all, prost is maintained by tokio.
Imo, we should stick with the popular and well-used lib for now and piggyback on them improving performance for having future optimizations like this.
Since both encode and decode are internal details, we can change them at any time without any breakage, so i am not too worried about this optimization for now.
Also, currently our messages have limited size and don't really contain any big blobs, so its upside is limited.
However it did motivate me to look into some other optimization which is to replace all vec with Bytes, which is easily doable and could avoid some vec clones at some of the places.
While in decode, prost has special handling for ref-counted byte buffers, it will not clone them, will return a ref to slice instead. Reqwest already returns ref-counted Bytes as response.
Also, I am not sure how much of improvement it is if any, we can send reqwest a Bytes object instead of vec and it can create body from that. In the current version of reqwest that we are using, stream trait for body isn't stable iiuc.
It mentions "// The Stream trait isn't stable, so the impl isn't public.".

Both of libs aren't maintained at all, prost is maintained by tokio. Imo, we should stick with the popular and well-used lib for now and piggyback on them improving performance for having future optimizations like this. Since both encode and decode are internal details, we can change them at any time without any breakage, so i am not too worried about this optimization for now. Also, currently our messages have limited size and don't really contain any big blobs, so its upside is limited.
Fair enough! Thanks for investigating though.
However it did motivate me to look into some other optimization which is to replace all vec with Bytes, which is easily doable and could avoid some vec clones at some of the places.
Which clones are avoided? Or are you saying in another PR?
While in decode, prost has special handling for ref-counted byte buffers, it will not clone them, will return a ref to slice instead. Reqwest already returns ref-counted Bytes as response.
Not sure how this is relevant. Aren't we talking about encoding objects that we've created in memory?
Also, I am not sure how much of improvement it is if any, we can send reqwest a Bytes object instead of vec and it can create body from that.
Isn't that just a wrapper around a Vec in how we are using it? Nothing needs to be shared, IIUC.
In the current version of reqwest that we are using, stream trait for body isn't stable iiuc.
It mentions "// The Stream trait isn't stable, so the impl isn't public.".
Hmm... I see it as an optional feature not unstable.

Which clones are avoided? Or are you saying in another PR?
Yes, mostly while creating these proto objects if we already have access to a byte slice or buffer.
Not sure how this is relevant. Aren't we talking about encoding objects that we've created in memory?
Reqwest returns Bytes, it will read raw bytes from wire and place them on buffer. if we change generated protos to use Bytes, it can refer to a slice from that buffer, whereas if we use vec it has to clone that part of memory and allocate while converting from slice iiuc.
Isn't that just a wrapper around a Vec in how we are using it? Nothing needs to be shared, IIUC.
Yeah this doesn't change much of anything. It is mostly some minor optimizations that bytes might do internally like buffer allocation/reuse or optimizing storage of small Bytes objects.
Hmm... I see it as an optional feature not unstable.
It was mentioned in code: https://github.com/seanmonstar/reqwest/blame/cf69fd4bfe22855d576497eb94e9eb549e742475/src/async_impl/body.rs#L22
Iiuc, support for streaming as public api was only exposed last week: seanmonstar/reqwest#2255

jkczyz · 2024-09-11T23:36:04Z

+	async fn post_request<Rq: Message, Rs: Message + Default>(
+ &self, request: &Rq, url: &str,
+	) -> Result<Rs, LdkNodeServerError> {
+ let request_body = request.encode_to_vec();


Both of libs aren't maintained at all, prost is maintained by tokio. Imo, we should stick with the popular and well-used lib for now and piggyback on them improving performance for having future optimizations like this. Since both encode and decode are internal details, we can change them at any time without any breakage, so i am not too worried about this optimization for now. Also, currently our messages have limited size and don't really contain any big blobs, so its upside is limited.
Fair enough! Thanks for investigating though.
However it did motivate me to look into some other optimization which is to replace all vec with Bytes, which is easily doable and could avoid some vec clones at some of the places.
Which clones are avoided? Or are you saying in another PR?
While in decode, prost has special handling for ref-counted byte buffers, it will not clone them, will return a ref to slice instead. Reqwest already returns ref-counted Bytes as response.
Not sure how this is relevant. Aren't we talking about encoding objects that we've created in memory?
Also, I am not sure how much of improvement it is if any, we can send reqwest a Bytes object instead of vec and it can create body from that.
Isn't that just a wrapper around a Vec in how we are using it? Nothing needs to be shared, IIUC.
In the current version of reqwest that we are using, stream trait for body isn't stable iiuc.
It mentions "// The Stream trait isn't stable, so the impl isn't public.".
Hmm... I see it as an optional feature not unstable.

jkczyz

LGTM. Please squash.

G8XSU · 2024-09-12T21:14:07Z

Rebased on main and squashed.

G8XSU changed the title ~~[Draft PR] Add base REST Client and CLI setup.~~Add base REST Client and CLI setup.Sep 10, 2024

G8XSU requested a review from jkczyz September 10, 2024 22:12

tnull reviewed Sep 11, 2024
View reviewed changes

Comment threadcli/Cargo.toml Outdated

jkczyz reviewed Sep 11, 2024
View reviewed changes

G8XSU requested a review from jkczyz September 12, 2024 17:58

jkczyz reviewed Sep 12, 2024
View reviewed changes

G8XSU added 10 commits September 12, 2024 14:12

Add base rest client setup.
0530b75

Add initial set of api methods to rest-client.
e453109

Add base cli setup.
b9d6b04

Add Cli command for OnchainSend
b685389

Add Cli command for Bolt11Receive
7546ae0

Add Cli command for Bolt11Send
d78d5e8

Add Cli command for Bolt12Receive
68473b5

Add Cli command for Bolt12Send
cbe6ea6

Add Cli command for OpenChannel
6a4825e

Exit with status code 1 on error.
2491cdf

jkczyz approved these changes Sep 12, 2024
View reviewed changes

G8XSU merged commit 06f986c into lightningdevkit:mainSep 12, 2024

joostjager mentioned this pull request Mar 28, 2026
Replace REST and RabbitMQ with gRPC #176
Merged

rsafier pushed a commit to rsafier/ldk-server that referenced this pull request Apr 20, 2026
Merge pull requestlightningdevkit#6from G8XSU/cli-base-setup
a85d5f4

G8XSU changed the title ~~[Draft PR] Add base REST Client and CLI setup.~~Add base REST Client and CLI setup.Sep 10, 2024

G8XSU requested a review from jkczyz September 10, 2024 22:12

tnull reviewed Sep 11, 2024
View reviewed changes

Comment threadcli/Cargo.toml Outdated

jkczyz reviewed Sep 11, 2024
View reviewed changes

G8XSU requested a review from jkczyz September 12, 2024 17:58

jkczyz reviewed Sep 12, 2024
View reviewed changes

G8XSU added 10 commits September 12, 2024 14:12

Add base rest client setup.
0530b75

Add initial set of api methods to rest-client.
e453109

Add base cli setup.
b9d6b04

Add Cli command for OnchainSend
b685389

Add Cli command for Bolt11Receive
7546ae0

Add Cli command for Bolt11Send
d78d5e8

Add Cli command for Bolt12Receive
68473b5

Add Cli command for Bolt12Send
cbe6ea6

Add Cli command for OpenChannel
6a4825e

Exit with status code 1 on error.
2491cdf

jkczyz approved these changes Sep 12, 2024
View reviewed changes

G8XSU merged commit 06f986c into lightningdevkit:mainSep 12, 2024

joostjager mentioned this pull request Mar 28, 2026
Replace REST and RabbitMQ with gRPC #176
Merged

rsafier pushed a commit to rsafier/ldk-server that referenced this pull request Apr 20, 2026
Merge pull requestlightningdevkit#6from G8XSU/cli-base-setup
a85d5f4

G8XSU changed the title ~~[Draft PR] Add base REST Client and CLI setup.~~Add base REST Client and CLI setup.Sep 10, 2024

G8XSU requested a review from jkczyz September 10, 2024 22:12

tnull reviewed Sep 11, 2024
View reviewed changes

Comment threadcli/Cargo.toml Outdated

jkczyz reviewed Sep 11, 2024
View reviewed changes

G8XSU requested a review from jkczyz September 12, 2024 17:58

jkczyz reviewed Sep 12, 2024
View reviewed changes

G8XSU added 10 commits September 12, 2024 14:12

Add base rest client setup.
0530b75

Add initial set of api methods to rest-client.
e453109

Add base cli setup.
b9d6b04

Add Cli command for OnchainSend
b685389

Add Cli command for Bolt11Receive
7546ae0

Add Cli command for Bolt11Send
d78d5e8

Add Cli command for Bolt12Receive
68473b5

Add Cli command for Bolt12Send
cbe6ea6

Add Cli command for OpenChannel
6a4825e

Exit with status code 1 on error.
2491cdf

jkczyz approved these changes Sep 12, 2024
View reviewed changes

G8XSU merged commit 06f986c into lightningdevkit:mainSep 12, 2024

joostjager mentioned this pull request Mar 28, 2026
Replace REST and RabbitMQ with gRPC #176
Merged

rsafier pushed a commit to rsafier/ldk-server that referenced this pull request Apr 20, 2026
Merge pull requestlightningdevkit#6from G8XSU/cli-base-setup
a85d5f4

G8XSU changed the title ~~[Draft PR] Add base REST Client and CLI setup.~~Add base REST Client and CLI setup.Sep 10, 2024

G8XSU requested a review from jkczyz September 10, 2024 22:12

tnull reviewed Sep 11, 2024
View reviewed changes

Comment threadcli/Cargo.toml Outdated

jkczyz reviewed Sep 11, 2024
View reviewed changes

G8XSU requested a review from jkczyz September 12, 2024 17:58

jkczyz reviewed Sep 12, 2024
View reviewed changes

G8XSU added 10 commits September 12, 2024 14:12

Add base rest client setup.
0530b75

Add initial set of api methods to rest-client.
e453109

Add base cli setup.
b9d6b04

Add Cli command for OnchainSend
b685389

Add Cli command for Bolt11Receive
7546ae0

Add Cli command for Bolt11Send
d78d5e8

Add Cli command for Bolt12Receive
68473b5

Add Cli command for Bolt12Send
cbe6ea6

Add Cli command for OpenChannel
6a4825e

Exit with status code 1 on error.
2491cdf

jkczyz approved these changes Sep 12, 2024
View reviewed changes

G8XSU merged commit 06f986c into lightningdevkit:mainSep 12, 2024

joostjager mentioned this pull request Mar 28, 2026
Replace REST and RabbitMQ with gRPC #176
Merged

rsafier pushed a commit to rsafier/ldk-server that referenced this pull request Apr 20, 2026
Merge pull requestlightningdevkit#6from G8XSU/cli-base-setup
a85d5f4

G8XSU changed the title ~~[Draft PR] Add base REST Client and CLI setup.~~Add base REST Client and CLI setup.Sep 10, 2024

G8XSU requested a review from jkczyz September 10, 2024 22:12

tnull reviewed Sep 11, 2024
View reviewed changes

Comment threadcli/Cargo.toml Outdated

jkczyz reviewed Sep 11, 2024
View reviewed changes

G8XSU requested a review from jkczyz September 12, 2024 17:58

jkczyz reviewed Sep 12, 2024
View reviewed changes

G8XSU added 10 commits September 12, 2024 14:12

Add base rest client setup.
0530b75

Add initial set of api methods to rest-client.
e453109

Add base cli setup.
b9d6b04

Add Cli command for OnchainSend
b685389

Add Cli command for Bolt11Receive
7546ae0

Add Cli command for Bolt11Send
d78d5e8

Add Cli command for Bolt12Receive
68473b5

Add Cli command for Bolt12Send
cbe6ea6

Add Cli command for OpenChannel
6a4825e

Exit with status code 1 on error.
2491cdf

jkczyz approved these changes Sep 12, 2024
View reviewed changes

G8XSU merged commit 06f986c into lightningdevkit:mainSep 12, 2024

joostjager mentioned this pull request Mar 28, 2026
Replace REST and RabbitMQ with gRPC #176
Merged

rsafier pushed a commit to rsafier/ldk-server that referenced this pull request Apr 20, 2026
Merge pull requestlightningdevkit#6from G8XSU/cli-base-setup
a85d5f4

G8XSU changed the title ~~[Draft PR] Add base REST Client and CLI setup.~~Add base REST Client and CLI setup.Sep 10, 2024

G8XSU requested a review from jkczyz September 10, 2024 22:12

tnull reviewed Sep 11, 2024
View reviewed changes

Comment threadcli/Cargo.toml Outdated

jkczyz reviewed Sep 11, 2024
View reviewed changes

G8XSU requested a review from jkczyz September 12, 2024 17:58

jkczyz reviewed Sep 12, 2024
View reviewed changes

G8XSU added 10 commits September 12, 2024 14:12

Add base rest client setup.
0530b75

Add initial set of api methods to rest-client.
e453109

Add base cli setup.
b9d6b04

Add Cli command for OnchainSend
b685389

Add Cli command for Bolt11Receive
7546ae0

Add Cli command for Bolt11Send
d78d5e8

Add Cli command for Bolt12Receive
68473b5

Add Cli command for Bolt12Send
cbe6ea6

Add Cli command for OpenChannel
6a4825e

Exit with status code 1 on error.
2491cdf

jkczyz approved these changes Sep 12, 2024
View reviewed changes

G8XSU merged commit 06f986c into lightningdevkit:mainSep 12, 2024

joostjager mentioned this pull request Mar 28, 2026
Replace REST and RabbitMQ with gRPC #176
Merged

rsafier pushed a commit to rsafier/ldk-server that referenced this pull request Apr 20, 2026
Merge pull requestlightningdevkit#6from G8XSU/cli-base-setup
a85d5f4

G8XSU changed the title ~~[Draft PR] Add base REST Client and CLI setup.~~Add base REST Client and CLI setup.Sep 10, 2024

G8XSU requested a review from jkczyz September 10, 2024 22:12

tnull reviewed Sep 11, 2024
View reviewed changes

Comment threadcli/Cargo.toml Outdated

jkczyz reviewed Sep 11, 2024
View reviewed changes

G8XSU requested a review from jkczyz September 12, 2024 17:58

jkczyz reviewed Sep 12, 2024
View reviewed changes

G8XSU added 10 commits September 12, 2024 14:12

Add base rest client setup.
0530b75

Add initial set of api methods to rest-client.
e453109

Add base cli setup.
b9d6b04

Add Cli command for OnchainSend
b685389

Add Cli command for Bolt11Receive
7546ae0

Add Cli command for Bolt11Send
d78d5e8

Add Cli command for Bolt12Receive
68473b5

Add Cli command for Bolt12Send
cbe6ea6

Add Cli command for OpenChannel
6a4825e

Exit with status code 1 on error.
2491cdf

jkczyz approved these changes Sep 12, 2024
View reviewed changes

G8XSU merged commit 06f986c into lightningdevkit:mainSep 12, 2024

joostjager mentioned this pull request Mar 28, 2026
Replace REST and RabbitMQ with gRPC #176
Merged

rsafier pushed a commit to rsafier/ldk-server that referenced this pull request Apr 20, 2026
Merge pull requestlightningdevkit#6from G8XSU/cli-base-setup
a85d5f4

G8XSU changed the title ~~[Draft PR] Add base REST Client and CLI setup.~~Add base REST Client and CLI setup.Sep 10, 2024

G8XSU requested a review from jkczyz September 10, 2024 22:12

tnull reviewed Sep 11, 2024
View reviewed changes

Comment threadcli/Cargo.toml Outdated

jkczyz reviewed Sep 11, 2024
View reviewed changes

G8XSU requested a review from jkczyz September 12, 2024 17:58

jkczyz reviewed Sep 12, 2024
View reviewed changes

G8XSU added 10 commits September 12, 2024 14:12

Add base rest client setup.
0530b75

Add initial set of api methods to rest-client.
e453109

Add base cli setup.
b9d6b04

Add Cli command for OnchainSend
b685389

Add Cli command for Bolt11Receive
7546ae0

Add Cli command for Bolt11Send
d78d5e8

Add Cli command for Bolt12Receive
68473b5

Add Cli command for Bolt12Send
cbe6ea6

Add Cli command for OpenChannel
6a4825e

Exit with status code 1 on error.
2491cdf

jkczyz approved these changes Sep 12, 2024
View reviewed changes

G8XSU merged commit 06f986c into lightningdevkit:mainSep 12, 2024

joostjager mentioned this pull request Mar 28, 2026
Replace REST and RabbitMQ with gRPC #176
Merged

rsafier pushed a commit to rsafier/ldk-server that referenced this pull request Apr 20, 2026
Merge pull requestlightningdevkit#6from G8XSU/cli-base-setup
a85d5f4

Uh oh!

Conversation

G8XSU commented Sep 6, 2024

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jkczyz left a comment

Choose a reason for hiding this comment

Uh oh!

G8XSU commented Sep 12, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

G8XSU commented Sep 6, 2024

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jkczyz left a comment

Choose a reason for hiding this comment

Uh oh!

G8XSU commented Sep 12, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

G8XSU commented Sep 6, 2024

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!