-
Notifications
You must be signed in to change notification settings - Fork 24
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
Setup grpc server #104
Setup grpc server #104
Conversation
Why do we need to need to pass the lnd connection params through to the grpc calls using grpc metadata, given that we already establish those through lndk's startup paramaters? |
@mrfelton This is actually something I went back and forth on, and would love some feedback. I landed on passing in the credentials because we need some sort of authentication mechanism for using lndk-cli anyway, and it seems reasonable to me to make sure that whoever's using lndk-cli is actually authorized to be using lnd. And since lnd already has those auth measures, it feels better to me to pass those credentials in here then to invent some new auth measures if it's not needed. You mentioned tls in Slack, which I agree is a must. But even with that, it feels wrong to me to not verify that the lndk-cli user has the needed lnd credentials. @carlaKC would love your thoughts on this as well when you have a moment -- though take your time. |
It would make more sense to actually secure the LNDK gRPC server with the lnd creds directly. The user story should be:
As it's implemented now, it's not adding any additional security since the only thing being passed in the grpc metadata is the paths to the creds, not the creds themselves. So one only has to guess what path the creds are stored at on the LNDK instance in order to be able to pay offers, and they are most likely stored in the default location. This is how it works currently: var channel = new(lndkUriWithPort, ChannelCredentials.Insecure);
var client = new OffersClient(channel),
var headers = new Metadata {
{"tls_cert_path", "/lnd/.lnd/tls.cert"},
{"macaroon_path", "/lnd/.lnd/data/chain/bitcoin/regtest/admin.macaroon"},
{"address", "https://olympus-lndus1:10009"}
};
var res = await client.Offers.PayOfferAsync(new PayOfferRequest {
Offer = offer,
Amount = amount,
}, headers: headers); Anyone can guess those paths. The caller should be required to pass the actual creds, so the client call should look more like: var certPem = "-----BEGIN CERTIFICATE-----\n...";
var macaroonHex = "0201036C6E6402F801...";
ChannelCredentials credentials = new SslCredentials(certPem);
var macaroonInterceptor = new AsyncAuthInterceptor((_, metadata) => {
metadata.Add(new("macaroon", macaroonHex));
return Task.CompletedTask;
});
credentials = ChannelCredentials.Create(credentials, CallCredentials.FromInterceptor(macaroonInterceptor));
var channel = new(lndkUriWithPort, credentials);
var client = new OffersClient(channel),
var res = await client.Offers.PayOfferAsync(new PayOfferRequest {
Offer = offer,
Amount = amount,
}); Either that, or LNDK should generate it's own tls cert and macaroon and those should be required to call it's gRPC api. |
A couple of things I noticed:
|
3f24b35
to
0a8d49b
Compare
0a8d49b
to
363fc82
Compare
This commit Arc-ifies the reference to OffersHandler, which is necessary because once we set up the grpc server, we'll need a second thread-safe reference to it.
This commit removes the RefCell in MessengerUtilities, which hampers us from being able to reference OfferHandler in two different tokio tasks, which we'll need to do when we set up our grpc server.
We move logger setup into main, so we're able to access the logger when we start up the server.
With the grpc server, we don't need to check whether the onion messenger has started before we pay an offer - we know it's already running.
That way, we don't get a "AlreadyProcessing" error if the client attempts to pay the offer a second time.
e424956
to
a3ed7dd
Compare
a3ed7dd
to
32b37f7
Compare
This updates the CLI to connect to the LNDK gRPC server in order to make API calls. We add the necessary macaroon metadata to each request. In addition, we remove the cert and address flag options because we no longer require passing in an LND certificate and address to connect to LND.
Co-authored-by: Tom <[email protected]>
32b37f7
to
eb321f3
Compare
This PR sets up a grpc server for interacting with a running LNDK instance. The main command it supports right now is to pay an offer.
In summary: