Skip to content
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

make Lnd properties public ? #36

Open
Asone opened this issue Jun 3, 2022 · 3 comments
Open

make Lnd properties public ? #36

Asone opened this issue Jun 3, 2022 · 3 comments

Comments

@Asone
Copy link
Contributor

Asone commented Jun 3, 2022

As i'm digging into the lib i'm trying to figure out why the Lnd properties are no directly accessible and exposed through some kind of proxy methods instead.

For example :

lnd-rs/src/lib.rs

Lines 211 to 227 in d1595d1

pub async fn list_invoices(
&mut self,
pending_only: bool,
index_offset: u64,
num_max_invoices: u64,
reversed: bool,
) -> Result<ListInvoiceResponse, Status> {
self.lightning
.list_invoices(ListInvoiceRequest {
pending_only,
index_offset,
num_max_invoices,
reversed,
})
.await
.map(Response::into_inner)
}

would be exactly the same as writing some kind of :

        let list_invoice_request = ListInvoiceRequest{
            pending_only: true,
            index_offset: 0 as u64,
            num_max_invoices: u64,
            reversed: false
        };

        let list_invoice_response = mylndclient.lightning.list_invoices(list_invoice_request).await.unwrap().into_inner();

if the sub-clients were publicly accessible.

I do undestand that the proxy methods unwraps the result of the requests to provide with the inner response directly, however i feel like a limitation to not have access to the direct clients as it makes use of the client impossible for the methods that are not proxified.

Maybe those clients could be made public which does not appear incompatible to providing the proxy methods ?

PR Proposal here : #37

@Asone
Copy link
Contributor Author

Asone commented Jul 14, 2022

Any take on this or should i close it ?

@Asone
Copy link
Contributor Author

Asone commented Dec 7, 2022

@lsunsi any chance to have an opinion onto this ?

@lsunsi
Copy link
Contributor

lsunsi commented Dec 7, 2022

@Asone Sorry for the delay on answering this, I saw it and got busy and forgot to answer. My bad!

My first thought on this is that the inner clients should be hidden from the user, in order for it to use the "lightning API" as thought it's just one client. Feels to me like the way lightning labs broke the protos is just an implementation detail.

This would mean we'd have to kind of rewrite all of the methods on the outer client, which is of course tedious. Do you think the major feature of exposing the inner clients is removing this tedious bit? Because if that's the case, maybe we could do with a macro or something? What do you think or have in mind?

Anyway, thanks for the interest in contributing and specially the patience, I'm sorry again for the delays, but let's discuss more about this!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants