From cd7e4f98c0d448c4f6079eba4a87ad5354674c5f Mon Sep 17 00:00:00 2001 From: Conner Fromknecht Date: Sat, 18 Aug 2018 17:40:09 -0700 Subject: [PATCH 1/4] htlcswitch/link_test: check fwdpkg cleanup for failed adds --- htlcswitch/link_test.go | 43 +++++++++++++++++++++++++++++++++-------- 1 file changed, 35 insertions(+), 8 deletions(-) diff --git a/htlcswitch/link_test.go b/htlcswitch/link_test.go index d261d8f67..bf1177bbb 100644 --- a/htlcswitch/link_test.go +++ b/htlcswitch/link_test.go @@ -1058,15 +1058,17 @@ func TestChannelLinkMultiHopUnknownNextHop(t *testing.T) { htlcAmt, totalTimelock, hops := generateHops(amount, testStartingHeight, n.firstBobChannelLink, n.carolChannelLink) - daveServer, err := newMockServer( - t, "dave", testStartingHeight, nil, n.globalPolicy.TimeLockDelta, + // Remove bob's outgoing link with Carol. This will cause him to fail + // back the payment to Alice since he is unaware of Carol when the + // payment comes across. + bobChanID := lnwire.NewChanIDFromOutPoint( + &channels.bobToCarol.State().FundingOutpoint, ) - if err != nil { - t.Fatalf("unable to init dave's server: %v", err) - } - davePub := daveServer.PubKey() - receiver := n.bobServer - rhash, err := n.makePayment(n.aliceServer, n.bobServer, davePub, hops, + n.bobServer.htlcSwitch.RemoveLink(bobChanID) + + bobPub := n.bobServer.PubKey() + receiver := n.carolServer + rhash, err := n.makePayment(n.aliceServer, receiver, bobPub, hops, amount, htlcAmt, totalTimelock).Wait(30 * time.Second) if err == nil { t.Fatal("error haven't been received") @@ -1108,6 +1110,31 @@ func TestChannelLinkMultiHopUnknownNextHop(t *testing.T) { t.Fatal("the bandwidth of carol channel link which handles " + "bob->carol channel should be the same") } + + // Load the forwarding packages for Bob's incoming link. The payment + // should have been rejected by the switch, and the AddRef in this link + // should be acked by the failed payment. + bobInFwdPkgs, err := channels.bobToAlice.State().LoadFwdPkgs() + if err != nil { + t.Fatalf("unable to load bob's fwd pkgs: %v", err) + } + + // There should be exactly two forward packages, as a full state + // transition requires two commitment dances. + if len(bobInFwdPkgs) != 2 { + t.Fatalf("bob should have exactly 2 fwdpkgs, has %d", + len(bobInFwdPkgs)) + } + + // Only one of the forwarding package should have an Add in it, the + // other will be empty. Either way, both AckFilters should be fully + // acked. + for _, fwdPkg := range bobInFwdPkgs { + if !fwdPkg.AckFilter.IsFull() { + t.Fatalf("fwdpkg chanid=%v height=%d AckFilter is not "+ + "fully acked", fwdPkg.Source, fwdPkg.Height) + } + } } // TestChannelLinkMultiHopDecodeError checks that we send HTLC cancel if From 4f2137eafc4f4a4f372d4c2792b4f482377ed89d Mon Sep 17 00:00:00 2001 From: Conner Fromknecht Date: Fri, 27 Jul 2018 03:46:56 -0700 Subject: [PATCH 2/4] htlcswitch/switch: set SourceRef of failed packets --- htlcswitch/switch.go | 1 + 1 file changed, 1 insertion(+) diff --git a/htlcswitch/switch.go b/htlcswitch/switch.go index 74d7f568d..afdd001cf 100644 --- a/htlcswitch/switch.go +++ b/htlcswitch/switch.go @@ -1174,6 +1174,7 @@ func (s *Switch) failAddPacket(packet *htlcPacket, log.Error(failErr) failPkt := &htlcPacket{ + sourceRef: packet.sourceRef, incomingChanID: packet.incomingChanID, incomingHTLCID: packet.incomingHTLCID, circuit: packet.circuit, From f636ff8b4ad48cf2c5c26feca11c97a97791e952 Mon Sep 17 00:00:00 2001 From: Conner Fromknecht Date: Mon, 20 Aug 2018 18:48:49 -0700 Subject: [PATCH 3/4] htlcswitch/test_utils: bump fwdpkg gc timeout to 15s This commit increases the fwdpkg garbage collection interval to 15s, to mitigate the likelihood of it interfering with our unit tests related to fwdpkgs. --- htlcswitch/test_utils.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/htlcswitch/test_utils.go b/htlcswitch/test_utils.go index 1e7c81828..bc95b6c1c 100644 --- a/htlcswitch/test_utils.go +++ b/htlcswitch/test_utils.go @@ -877,7 +877,7 @@ func newThreeHopNetwork(t testing.TB, aliceChannel, firstBobChannel, const ( batchTimeout = 50 * time.Millisecond - fwdPkgTimeout = 5 * time.Second + fwdPkgTimeout = 15 * time.Second minFeeUpdateTimeout = 30 * time.Minute maxFeeUpdateTimeout = 40 * time.Minute ) From 53b58a1eb3f0a06e04edf1b46aceaa46f6e7303c Mon Sep 17 00:00:00 2001 From: Conner Fromknecht Date: Mon, 20 Aug 2018 19:23:15 -0700 Subject: [PATCH 4/4] htlcswitch/mock: drop messages if link is not online In this commit, we modify the readHandler w/in the mock peer to drop messages if it is unable to find the target link. This has led to observed race conditions related to removing a link and still attempting to deliver messages. By removing this, the readHandler shouldn't fail the test as a result. --- htlcswitch/mock.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/htlcswitch/mock.go b/htlcswitch/mock.go index 64f843fc0..9df919933 100644 --- a/htlcswitch/mock.go +++ b/htlcswitch/mock.go @@ -500,11 +500,12 @@ func (s *mockServer) readHandler(message lnwire.Message) error { return fmt.Errorf("unknown message type: %T", msg) } - // Dispatch the commitment update message to the proper - // channel link dedicated to this channel. + // Dispatch the commitment update message to the proper channel link + // dedicated to this channel. If the link is not found, we will discard + // the message. link, err := s.htlcSwitch.GetLink(targetChan) if err != nil { - return err + return nil } // Create goroutine for this, in order to be able to properly stop