From a779befda5bc6433318b1ae1b8768dd4f4349b42 Mon Sep 17 00:00:00 2001 From: Conner Fromknecht Date: Sat, 9 Jun 2018 04:28:03 -0700 Subject: [PATCH 1/2] htlcswitch/switch_test: adds duplicate link add test --- htlcswitch/switch_test.go | 59 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 59 insertions(+) diff --git a/htlcswitch/switch_test.go b/htlcswitch/switch_test.go index 82932d3c..852aa654 100644 --- a/htlcswitch/switch_test.go +++ b/htlcswitch/switch_test.go @@ -25,6 +25,65 @@ func genPreimage() ([32]byte, error) { return preimage, nil } +// TestSwitchAddDuplicateLink tests that the switch will reject duplicate links +// for both pending and live links. It also tests that we can successfully +// add a link after having removed it. +func TestSwitchAddDuplicateLink(t *testing.T) { + t.Parallel() + + alicePeer, err := newMockServer(t, "alice", nil) + if err != nil { + t.Fatalf("unable to create alice server: %v", err) + } + + s, err := initSwitchWithDB(nil) + if err != nil { + t.Fatalf("unable to init switch: %v", err) + } + if err := s.Start(); err != nil { + t.Fatalf("unable to start switch: %v", err) + } + defer s.Stop() + + chanID1, _, aliceChanID, _ := genIDs() + + pendingChanID := lnwire.ShortChannelID{} + + aliceChannelLink := newMockChannelLink( + s, chanID1, pendingChanID, alicePeer, false, + ) + if err := s.AddLink(aliceChannelLink); err != nil { + t.Fatalf("unable to add alice link: %v", err) + } + + // Alice should have a pending link, adding again should fail. + if err := s.AddLink(aliceChannelLink); err == nil { + t.Fatalf("adding duplicate link should have failed") + } + + // Update the short chan id of the channel, so that the link goes live. + aliceChannelLink.setLiveShortChanID(aliceChanID) + err = s.UpdateShortChanID(chanID1) + if err != nil { + t.Fatalf("unable to update alice short_chan_id: %v", err) + } + + // Alice should have a live link, adding again should fail. + if err := s.AddLink(aliceChannelLink); err == nil { + t.Fatalf("adding duplicate link should have failed") + } + + // Remove the live link to ensure the indexes are cleared. + if err := s.RemoveLink(chanID1); err != nil { + t.Fatalf("unable to remove alice link: %v", err) + } + + // Alice has no links, adding should succeed. + if err := s.AddLink(aliceChannelLink); err != nil { + t.Fatalf("unable to add alice link: %v", err) + } +} + // TestSwitchSendPending checks the inability of htlc switch to forward adds // over pending links, and the UpdateShortChanID makes a pending link live. func TestSwitchSendPending(t *testing.T) { From e5233c8ec7dca631fde37d9840b5e09f24a25953 Mon Sep 17 00:00:00 2001 From: Conner Fromknecht Date: Sat, 9 Jun 2018 04:29:03 -0700 Subject: [PATCH 2/2] htlcswitch/switch: reject duplicate links, purge link indexes --- htlcswitch/switch.go | 51 ++++++++++++++++++++++---------------------- 1 file changed, 26 insertions(+), 25 deletions(-) diff --git a/htlcswitch/switch.go b/htlcswitch/switch.go index f11d1fd3..dcb6714d 100644 --- a/htlcswitch/switch.go +++ b/htlcswitch/switch.go @@ -1723,6 +1723,13 @@ func (s *Switch) AddLink(link ChannelLink) error { chanID := link.ChanID() + // First, ensure that this link is not already active in the switch. + _, err := s.getLink(chanID) + if err == nil { + return fmt.Errorf("unable to add ChannelLink(%v), already "+ + "active", chanID) + } + // Get and attach the mailbox for this link, which buffers packets in // case there packets that we tried to deliver while this link was // offline. @@ -1772,24 +1779,18 @@ func (s *Switch) addLiveLink(link ChannelLink) { s.interfaceIndex[peerPub][link] = struct{}{} } -// removeLiveLink removes a link from all associated forwarding indexes, this -// prevents it from being a candidate in forwarding. -func (s *Switch) removeLiveLink(link ChannelLink) { - // Remove the channel from live link indexes. - delete(s.linkIndex, link.ChanID()) - delete(s.forwardingIndex, link.ShortChanID()) - - // Remove the channel from channel index. - peerPub := link.Peer().PubKey() - delete(s.interfaceIndex, peerPub) -} - // GetLink is used to initiate the handling of the get link command. The // request will be propagated/handled to/in the main goroutine. func (s *Switch) GetLink(chanID lnwire.ChannelID) (ChannelLink, error) { s.indexMtx.RLock() defer s.indexMtx.RUnlock() + return s.getLink(chanID) +} + +// getLink returns the link stored in either the pending index or the live +// lindex. +func (s *Switch) getLink(chanID lnwire.ChannelID) (ChannelLink, error) { link, ok := s.linkIndex[chanID] if !ok { link, ok = s.pendingLinkIndex[chanID] @@ -1829,23 +1830,23 @@ func (s *Switch) RemoveLink(chanID lnwire.ChannelID) error { func (s *Switch) removeLink(chanID lnwire.ChannelID) error { log.Infof("Removing channel link with ChannelID(%v)", chanID) - link, ok := s.linkIndex[chanID] - if ok { - s.removeLiveLink(link) - link.Stop() - - return nil + link, err := s.getLink(chanID) + if err != nil { + return err } - link, ok = s.pendingLinkIndex[chanID] - if ok { - delete(s.pendingLinkIndex, chanID) - link.Stop() + // Remove the channel from live link indexes. + delete(s.pendingLinkIndex, link.ChanID()) + delete(s.linkIndex, link.ChanID()) + delete(s.forwardingIndex, link.ShortChanID()) - return nil - } + // Remove the channel from channel index. + peerPub := link.Peer().PubKey() + delete(s.interfaceIndex, peerPub) - return ErrChannelLinkNotFound + link.Stop() + + return nil } // UpdateShortChanID updates the short chan ID for an existing channel. This is