From 4b57fb199fe1f29d6876b0bdf7eb398e3ea577c0 Mon Sep 17 00:00:00 2001 From: Olaoluwa Osuntokun Date: Tue, 9 Apr 2019 19:45:48 -0700 Subject: [PATCH 1/4] server: add new errPeerAlreadyConnected error type --- server.go | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/server.go b/server.go index 81bb55c9..85bc531b 100644 --- a/server.go +++ b/server.go @@ -89,6 +89,19 @@ var ( validColorRegexp = regexp.MustCompile("^#[A-Fa-f0-9]{6}$") ) +// errPeerAlreadyConnected is an error returned by the server when we're +// commanded to connect to a peer, but they're already connected. +type errPeerAlreadyConnected struct { + peer *peer +} + +// Error returns the human readable version of this error type. +// +// NOTE: Part of the error interface. +func (e *errPeerAlreadyConnected) Error() string { + return fmt.Sprintf("already connected to peer: %v", e.peer) +} + // server is the main server of the Lightning Network Daemon. The server houses // global state pertaining to the wallet, database, and the rpcserver. // Additionally, the server is also used as a central messaging bus to interact @@ -2875,7 +2888,7 @@ func (s *server) ConnectToPeer(addr *lnwire.NetAddress, perm bool) error { peer, err := s.findPeerByPubStr(targetPub) if err == nil { s.mu.Unlock() - return fmt.Errorf("already connected to peer: %v", peer) + return &errPeerAlreadyConnected{peer: peer} } // Peer was not found, continue to pursue connection with peer. From 71e080a2fb5c6d6b5fb0ed11e9f9bc0d0fdd1750 Mon Sep 17 00:00:00 2001 From: Olaoluwa Osuntokun Date: Tue, 9 Apr 2019 19:47:00 -0700 Subject: [PATCH 2/4] chanrestore: don't exit with an error if we're already connected to the peer In this commit, we fix a bug in the existing logic for ConnectPeer that would cause an SCB restore to fail if we were already connected to the peer. To fix this, we now instead will just return with a success if we're already connected to the peer. --- chanrestore.go | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/chanrestore.go b/chanrestore.go index 7aaada2f..c1611395 100644 --- a/chanrestore.go +++ b/chanrestore.go @@ -171,7 +171,16 @@ func (s *server) ConnectPeer(nodePub *btcec.PublicKey, addrs []net.Addr) error { // Attempt to connect to the peer using this full address. If // we're unable to connect to them, then we'll try the next // address in place of it. - if err := s.ConnectToPeer(netAddr, true); err != nil { + err := s.ConnectToPeer(netAddr, true) + + // If we're already connected to this peer, then we don't + // consider this an erorr, so we'll exit here. + if _, ok := err.(*errPeerAlreadyConnected); ok { + return nil + + } else if err != nil { + // Otherwise, something else happened, so we'll try the + // next address. ltndLog.Errorf("unable to connect to %v to "+ "complete SCB restore: %v", netAddr, err) continue From 5ac9ba6472c4ed89a5ac8565a1f02b4289cbc82d Mon Sep 17 00:00:00 2001 From: Olaoluwa Osuntokun Date: Tue, 9 Apr 2019 19:48:34 -0700 Subject: [PATCH 3/4] htlcswitch: synchronously send the chan sync message to the remote peer In this commit, we modify the starting link logic to always send the chan sync message to the remote peer in a synchronous manner. Otherwise, it's possible that we fail very quickly below this block, and don't ever send the message to the remote peer. --- htlcswitch/link.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/htlcswitch/link.go b/htlcswitch/link.go index 076415b2..d6041266 100644 --- a/htlcswitch/link.go +++ b/htlcswitch/link.go @@ -574,7 +574,7 @@ func (l *channelLink) syncChanStates() error { return fmt.Errorf("unable to generate chan sync message for "+ "ChannelPoint(%v)", l.channel.ChannelPoint()) } - if err := l.cfg.Peer.SendMessage(false, localChanSyncMsg); err != nil { + if err := l.cfg.Peer.SendMessage(true, localChanSyncMsg); err != nil { return fmt.Errorf("Unable to send chan sync message for "+ "ChannelPoint(%v)", l.channel.ChannelPoint()) } From 7b0a31217f5db2bcd5e33a30648a75b21b6a1040 Mon Sep 17 00:00:00 2001 From: Olaoluwa Osuntokun Date: Wed, 10 Apr 2019 17:11:19 -0700 Subject: [PATCH 4/4] chanrestore: ensure we have no existing connections to recovery peer In this commit, we fix a slight bug in the existing implantation that would cause no channel recovery if the recovering node was already connected to their channel peer. As we need the link to be known at the time of connection, if we're already connected, then the chan sync message won't be sent again. By first disconnecting an existing peer, we ensure that during the next connection (after the recovered channel is added to the database), then the regular chan sync message exchange will take place as expected. # Please enter the commit message for your changes. --- chanrestore.go | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/chanrestore.go b/chanrestore.go index c1611395..bb1ba680 100644 --- a/chanrestore.go +++ b/chanrestore.go @@ -156,6 +156,14 @@ var _ chanbackup.ChannelRestorer = (*chanDBRestorer)(nil) // // NOTE: Part of the chanbackup.PeerConnector interface. func (s *server) ConnectPeer(nodePub *btcec.PublicKey, addrs []net.Addr) error { + // Before we connect to the remote peer, we'll remove any connections + // to ensure the new connection is created after this new link/channel + // is known. + if err := s.DisconnectPeer(nodePub); err != nil { + ltndLog.Infof("Peer(%v) is already connected, proceeding "+ + "with chan restore", nodePub.SerializeCompressed()) + } + // For each of the known addresses, we'll attempt to launch a // persistent connection to the (pub, addr) pair. In the event that any // of them connect, all the other stale requests will be cancelled. @@ -174,7 +182,7 @@ func (s *server) ConnectPeer(nodePub *btcec.PublicKey, addrs []net.Addr) error { err := s.ConnectToPeer(netAddr, true) // If we're already connected to this peer, then we don't - // consider this an erorr, so we'll exit here. + // consider this an error, so we'll exit here. if _, ok := err.(*errPeerAlreadyConnected); ok { return nil