multi: switch to lndclient and fail on errors#58
multi: switch to lndclient and fail on errors#58Roasbeef merged 2 commits intolightninglabs:masterfrom
Conversation
Roasbeef
left a comment
There was a problem hiding this comment.
Change reads well, also will be nice to have the system properly restart if any of the components go down. One thing I'm weary of is the lndclient dep, though maybe in practice this won't be an issue since we don't really frequently update this repo like some of our others.
| "context" | ||
|
|
||
| "github.com/lightningnetwork/lnd/lnrpc" | ||
| "github.com/lightninglabs/lndclient" |
There was a problem hiding this comment.
Hmm, will switching over to using lndclient actually cause more issues than it solves? Main thing I'm weary of is the need to update lndclient to expose some new functionality, rather than just updating the lnd package commit, then immediately having access to the new features.
There was a problem hiding this comment.
I haven't found that using lndclient has been much of a blocker for loop/faraday, the lndclient PRs are pretty straightforward to review and I do think we get some degree of increasing returns when we add endpoints to lndclient because they're then available for other projects. We can do this change without a switchover ofc, so happy to remove it if you think it'll slow dev too much, but I think it's worth doing.
There was a problem hiding this comment.
Ok np, if y'all don't think it slows things down, then I'm ok with the switch.
|
|
||
| // Logger for lndmon's main process. | ||
| Logger = backendLog.Logger("LNDMON") | ||
|
|
There was a problem hiding this comment.
These loggers were only used to log errors when we couldn't query lnd, which is now replaced by bubbling the error up through errChan. I don't love deleting them, because we could potentially re-add them in future with different prefixes, which would be inconsistent for end users, but the linter is complaining. Could add some placeholder log for each collector so we don't need to delete?
Switch lndmon to use lndclient, providing the readonly macaroon as our only macaroon, so that no changes to lndmon setup are required. This switch also provides us with version checks, which we set to lnd v0.11, since that is the minimum supported version after recent CVEs.
Update all of our collectors to shutdown on failure rather than sliently log. This paired with restarting the lndmon container on exit allows easier detection of persistenet issues, and simple restart when lnd is unavailable temporarily.
This PR does some prepwork for adding the output from the htlc subscription api to lndmon, as described in #57.
Since we will need access to
routerrpcforSubscribeHtlcEvents, this PR takes the plunge and switches us over to the full lndclient services. Rather than require all macaroons for lndmon, this PR depends on lightninglabs/lndclient/pull/19, which allows us to specify a single macaroon for lndclient.This PR also updates lndmon to restart on collector failure, rather than just log an error.