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

healthcheck: make sure chain backend has enough outbound peers #8487

Closed
yyforyongyu opened this issue Feb 18, 2024 · 5 comments · Fixed by #8576
Closed

healthcheck: make sure chain backend has enough outbound peers #8487

yyforyongyu opened this issue Feb 18, 2024 · 5 comments · Fixed by #8576
Labels
beginner Issues suitable for new developers chain handling enhancement Improvements to existing features / behaviour good first issue Issues suitable for first time contributors to LND healthcheck P3 might get fixed, nice to have security General label for issues/PRs related to the security of the software

Comments

@yyforyongyu
Copy link
Collaborator

Resulted from a discussion in #8418, we should have a health check on the number of outbound peers to make sure bitcoind maintains a healthy connection to the network, which is straight forward,

  • use GetPeerInfo to get the peers info and count the outbounds.
  • if the number is below 6, log warnings.
  • if the number is below 2, maybe stop lnd?
@yyforyongyu yyforyongyu added enhancement Improvements to existing features / behaviour security General label for issues/PRs related to the security of the software chain handling healthcheck labels Feb 18, 2024
@yyforyongyu yyforyongyu added beginner Issues suitable for new developers good first issue Issues suitable for first time contributors to LND labels Feb 18, 2024
@Crypt-iQ
Copy link
Collaborator

I don't think we should stop lnd if we have below two outbound peers. bitcoind should recover from this automatically and this would make lnd stop in some benign situations

@saubyk saubyk added the P3 might get fixed, nice to have label Feb 26, 2024
@srikanth-iyengar
Copy link

I would like to work on this issue,
So just to confirm we don't have to stop the lnd if the out bound peers drop below 2 right ?

@Crypt-iQ
Copy link
Collaborator

I don't think this issue has any actual use other than logging for something that we're not going to fix? If they're using bitcoind then bitcoind will solve this automatically.

@srikanth-iyengar
Copy link

Oh....then we are just going to log, will go ahead then

@srikanth-iyengar
Copy link

srikanth-iyengar commented Mar 14, 2024

I have a question,
I was planning to create a separate method for getting the peerCounts which will have the following signature in healthcheck/peercheck.go

func GetOutboundPeers(bitcoinNode string, bitcoind *lncfg.Bitcoind, neutrino *lncfg.Neutrino, btcd *lncfg.Btcd) {
    // prepare a rpcConfig using a simple switch
    client, err := rpcclient.New(&rcpconfig, nil)
    if err != nil {
        return fmt.Errorf("Error creating rpcclient %w", err)
    }
    // return the number of peers
}

But the problem is healthcheck is completely a different module and I cannot simply import lncfg,

If we cannot have lncfg in healthcheck how should I proceed further ?

One way I could think of is, Create a simple utils method somewhere within the lnd module which will take the *Config as param and will return the outbound peers, then I can simply call that method and log according to the number of peers

should I proceed with my proposed idea or am I like, totally messing up here?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
beginner Issues suitable for new developers chain handling enhancement Improvements to existing features / behaviour good first issue Issues suitable for first time contributors to LND healthcheck P3 might get fixed, nice to have security General label for issues/PRs related to the security of the software
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants