From f7681cb51df90a5fcaf9f0d6e2707ac5f30f8a28 Mon Sep 17 00:00:00 2001 From: eugene Date: Tue, 21 Jun 2022 16:28:00 -0400 Subject: [PATCH 1/3] server.go: replace call to removePeer with Disconnect in DisconnectPeer Without this, calls to DisconnectPeer would bypass the peerTerminationWatcher and allow subsequent connect requests to go through before the peer's links were fully shut down. This could lead to force closes. --- server.go | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/server.go b/server.go index 7ee149fbb..90cf60444 100644 --- a/server.go +++ b/server.go @@ -4217,11 +4217,9 @@ func (s *server) DisconnectPeer(pubKey *btcec.PublicKey) error { delete(s.persistentPeers, pubStr) delete(s.persistentPeersBackoff, pubStr) - // Remove the current peer from the server's internal state and signal - // that the peer termination watcher does not need to execute for this - // peer. - s.removePeer(peer) - s.ignorePeerTermination[peer] = struct{}{} + // Remove the peer by calling Disconnect. Previously this was done with + // removePeer, which bypassed the peerTerminationWatcher. + peer.Disconnect(fmt.Errorf("server: DisconnectPeer called")) return nil } From a7f5b89cf305e7e20dd032bcfdee930c67704ffe Mon Sep 17 00:00:00 2001 From: eugene Date: Wed, 22 Jun 2022 12:07:59 -0400 Subject: [PATCH 2/3] itest: update switch delivery tests with more robust reconnect logic --- lntest/itest/lnd_switch_test.go | 23 ++++++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/lntest/itest/lnd_switch_test.go b/lntest/itest/lnd_switch_test.go index 4ca5aa842..e40a405a4 100644 --- a/lntest/itest/lnd_switch_test.go +++ b/lntest/itest/lnd_switch_test.go @@ -9,6 +9,7 @@ import ( "github.com/lightningnetwork/lnd/lnrpc" "github.com/lightningnetwork/lnd/lntest" "github.com/lightningnetwork/lnd/lntest/wait" + "github.com/stretchr/testify/require" ) // testSwitchCircuitPersistence creates a multihop network to ensure the sender @@ -451,13 +452,23 @@ func testSwitchOfflineDelivery(net *lntest.NetworkHarness, t *harnessTest) { t.Fatalf("htlc mismatch: %v", predErr) } + peerReq := &lnrpc.PeerEventSubscription{} + peerClient, err := dave.SubscribePeerEvents(ctxb, peerReq) + require.NoError(t.t, err) + // First, disconnect Dave and Alice so that their link is broken. if err := net.DisconnectNodes(dave, net.Alice); err != nil { t.Fatalf("unable to disconnect alice from dave: %v", err) } + // Wait to receive the PEER_OFFLINE event before reconnecting them. + peerEvent, err := peerClient.Recv() + require.NoError(t.t, err) + require.Equal(t.t, lnrpc.PeerEvent_PEER_OFFLINE, peerEvent.GetType()) + // Then, reconnect them to ensure Dave doesn't just fail back the htlc. - net.ConnectNodes(t.t, dave, net.Alice) + // We use EnsureConnected here in case they have already re-connected. + net.EnsureConnected(t.t, dave, net.Alice) // Wait to ensure that the payment remain are not failed back after // reconnecting. All node should report the number payments initiated @@ -476,6 +487,16 @@ func testSwitchOfflineDelivery(net *lntest.NetworkHarness, t *harnessTest) { t.Fatalf("unable to disconnect alice from dave: %v", err) } + // Wait to receive the PEER_ONLINE and then the PEER_OFFLINE event + // before advancing. + peerEvent2, err := peerClient.Recv() + require.NoError(t.t, err) + require.Equal(t.t, lnrpc.PeerEvent_PEER_ONLINE, peerEvent2.GetType()) + + peerEvent3, err := peerClient.Recv() + require.NoError(t.t, err) + require.Equal(t.t, lnrpc.PeerEvent_PEER_OFFLINE, peerEvent3.GetType()) + // Now restart carol without hodl mode, to settle back the outstanding // payments. carol.SetExtraArgs(nil) From ff39f1e5eaac81fdbdf9f325e6eb3a0a2e99ac96 Mon Sep 17 00:00:00 2001 From: eugene Date: Wed, 29 Jun 2022 13:49:32 -0400 Subject: [PATCH 3/3] release-notes: update for 0.15.1 --- docs/release-notes/release-notes-0.15.1.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/docs/release-notes/release-notes-0.15.1.md b/docs/release-notes/release-notes-0.15.1.md index 410eb29ae..052a60a3a 100644 --- a/docs/release-notes/release-notes-0.15.1.md +++ b/docs/release-notes/release-notes-0.15.1.md @@ -34,10 +34,14 @@ * [Update the urfave/cli package](https://github.com/lightningnetwork/lnd/pull/6682) because of a flag parsing bug. +* [DisconnectPeer no longer interferes with the peerTerminationWatcher. This previously caused + force closes.](https://github.com/lightningnetwork/lnd/pull/6655) + # Contributors (Alphabetical Order) * Elle Mouton * ErikEk +* Eugene Siegel * Oliver Gugger * Priyansh Rastogi * Yong Yu \ No newline at end of file