Closed orbitalturtle closed 3 months ago
Attention: Patch coverage is 0%
with 1 line
in your changes missing coverage. Please review.
Project coverage is 0.00%. Comparing base (
7f9e0f9
) to head (b3f9c30
). Report is 4 commits behind head on master.
Files | Patch % | Lines |
---|---|---|
src/main.rs | 0.00% | 1 Missing :warning: |
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
The TLS IP stuff is working well. One observation with it was that the TLS cert was automatically regenerated when we restarted with the ip settings in place. It might be an idea to follow the model of how this works in lnd - which is to only regenerate the cert after deleting the existing ones from the file system, regardless of wether the startup options have changed. This provides a level of stability ensuring that in use tls certs don't get invalidated without explicit action to trigger that (deleting the old ones and restarting the node),
@mrfelton I just did some tests, and I believe that is how it works? It should only regenerate the certificate if either the key or certificate in ~/.lndk have been deleted: https://github.com/lndk-org/lndk/blob/c505ca46a5b3b0c1a35399f53df1520514330d07/src/server.rs#L335. (Though I think we should add a comment to the tls-ip config option explaining that this is the case - if a cert already exists, it must be deleted before it will regenerate.)
Retested and confirmed that the cert does only get regenerated if it's deleted from the disk.
I added an extra commit in https://github.com/orbitalturtle/lndk/pull/10 which adds the payer note into the fetch/decode invoice response data.
Looks like when no payer note is provided, it's actually setting the payer note on the invoice request as an empty string which I don't think is correct. The resulting invoice has payer_note: Some("")
when it probably should have payer_note: None
@mrfelton Yeah good point. I wasn't sure how to do that without getting a move error, but Jeff from LDK showed me how they do it on their end. Just pushed up a version with that change.
Just added the payer_note to the decoded invoice as well.
This PR includes a few smaller requested changes in one PR:
tls-ip
config option for specifying other ip addresses/domains. Multiple can be added if separated by commas.pay-offer
andget-invoice
commands.Closes #134.