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

server.go: replace call to removePeer with Disconnect in DisconnectPeer #6655

Merged
merged 3 commits into from
Jun 30, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions docs/release-notes/release-notes-0.15.1.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
23 changes: 22 additions & 1 deletion lntest/itest/lnd_switch_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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)
Expand Down
8 changes: 3 additions & 5 deletions server.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"))
yyforyongyu marked this conversation as resolved.
Show resolved Hide resolved

return nil
}
Expand Down