lightninglabs / neutrino

Privacy-Preserving Bitcoin Light Client
MIT License
907 stars 182 forks source link

NewChainService does not traverse through all supplied Neutrino Peers before returning with error #60

Open biscottigelato opened 6 years ago

biscottigelato commented 6 years ago

With multiple neutrino peers supplied through lnd.conf/neutrino.Config structure, it seems that NewChainService() will try to resolve the peers' addresses and try to connect to them. However as soon as one of these addresses are deemed invalid, NewChainService() stops attempting and simply returns with error immediately, without trying the remaining peer addresses.

At first glance it can be a fairly trivial change to remedy this? But not sure if there are other implications, like whether the neutrino.Config structure should be updated to remove the invalid addresses, etc.

        // Start up persistent peers.
    permanentPeers := cfg.ConnectPeers
    if len(permanentPeers) == 0 {
        permanentPeers = cfg.AddPeers
    }

    var peerConnErr error
    var peersConnected int

    for _, addr := range permanentPeers {
        tcpAddr, err := s.addrStringToNetAddr(addr)

        if err != nil {
            peerConnErr = err
        } else {
            peersConnected++
            go s.connManager.Connect(&connmgr.ConnReq{
                Addr:      tcpAddr,
                Permanent: true,
            })
        }
    }

    if peersConnected != 0 {
        peerConnErr = nil
    }

    return &s, peerConnErr

I can create a Pull Request if this seems like that right direction to go?

Roasbeef commented 4 years ago

Approach looks sound if you still want to pick this up. Atm, it's implemented more as an "all or nothing". I think this is also related to https://github.com/lightningnetwork/lnd/issues/4016