From daeeca0bc357ae729d4631d2e88fc61fcb39c4ea Mon Sep 17 00:00:00 2001 From: "Johan T. Halseth" Date: Fri, 13 Apr 2018 16:50:31 +0200 Subject: [PATCH 1/3] rpcserver: make copy of htlc rhash before returning in listchannels This commit fixes a bug where all the HTLC rhash slices in a ListChannelsResponse would be tied to the loop variable, making them all take the hash of the last HTLC in the list. This commit fixes it by making a copy of the slice. --- rpcserver.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/rpcserver.go b/rpcserver.go index 14cce2d3..601a338e 100644 --- a/rpcserver.go +++ b/rpcserver.go @@ -1636,10 +1636,12 @@ func (r *rpcServer) ListChannels(ctx context.Context, } for i, htlc := range localCommit.Htlcs { + var rHash [32]byte + copy(rHash[:], htlc.RHash[:]) channel.PendingHtlcs[i] = &lnrpc.HTLC{ Incoming: htlc.Incoming, Amount: int64(htlc.Amt.ToSatoshis()), - HashLock: htlc.RHash[:], + HashLock: rHash[:], ExpirationHeight: htlc.RefundTimeout, } } From 4bd45b22eba476be8259dd086c1d617c5d977701 Mon Sep 17 00:00:00 2001 From: "Johan T. Halseth" Date: Fri, 13 Apr 2018 17:14:42 +0200 Subject: [PATCH 2/3] lnd_test: make assertActiveHtlcs correctly check the exact pay hashes --- lnd_test.go | 38 ++++++++++++++++++++++---------------- 1 file changed, 22 insertions(+), 16 deletions(-) diff --git a/lnd_test.go b/lnd_test.go index 6f9d476a..c758b131 100644 --- a/lnd_test.go +++ b/lnd_test.go @@ -5341,6 +5341,8 @@ func testBidirectionalAsyncPayments(net *lntest.NetworkHarness, t *harnessTest) closeChannelAndAssert(ctxt, t, net, net.Alice, chanPoint, false) } +// assertActiveHtlcs makes sure all the passed nodes have the _exact_ HTLCs +// matching payHashes on _all_ their channels. func assertActiveHtlcs(nodes []*lntest.HarnessNode, payHashes ...[]byte) error { req := &lnrpc.ListChannelsRequest{} ctxb := context.Background() @@ -5351,27 +5353,31 @@ func assertActiveHtlcs(nodes []*lntest.HarnessNode, payHashes ...[]byte) error { } for _, channel := range nodeChans.Channels { - if len(channel.PendingHtlcs) == 0 { - return fmt.Errorf("node %x has no htlcs: %v", - node.PubKey[:], spew.Sdump(channel)) + // Record all payment hashes active for this channel. + htlcHashes := make(map[string]struct{}) + for _, htlc := range channel.PendingHtlcs { + _, ok := htlcHashes[string(htlc.HashLock)] + if ok { + return fmt.Errorf("duplicate HashLock") + } + htlcHashes[string(htlc.HashLock)] = struct{}{} } - for _, htlc := range channel.PendingHtlcs { + // Channel should have exactly the payHashes active. + if len(payHashes) != len(htlcHashes) { + return fmt.Errorf("node %x had %v htlcs active, "+ + "expected %v", node.PubKey[:], + len(htlcHashes), len(payHashes)) + } - var htlcIsMatch bool - for _, payHash := range payHashes { - if bytes.Equal(htlc.HashLock, payHash) { - htlcIsMatch = true - } - } - - if htlcIsMatch { + // Make sure all the payHashes are active. + for _, payHash := range payHashes { + if _, ok := htlcHashes[string(payHash)]; ok { continue } - - return fmt.Errorf("node %x doesn't have expected "+ - "payment hashes: %v", node.PubKey[:], - spew.Sdump(channel.PendingHtlcs)) + return fmt.Errorf("node %x didn't have the "+ + "payHash %v active", node.PubKey[:], + payHash) } } } From 6573eed37b1674ac5937e9f493ec638e87cbd5a9 Mon Sep 17 00:00:00 2001 From: "Johan T. Halseth" Date: Sun, 15 Apr 2018 18:55:55 +0200 Subject: [PATCH 3/3] lnd_test: wrap check for expected number of channels in WaitPredicate --- lnd_test.go | 45 ++++++++++++++++++++++++++++++--------------- 1 file changed, 30 insertions(+), 15 deletions(-) diff --git a/lnd_test.go b/lnd_test.go index c758b131..fed4dc02 100644 --- a/lnd_test.go +++ b/lnd_test.go @@ -3063,21 +3063,36 @@ func testPrivateChannels(net *lntest.NetworkHarness, t *harnessTest) { return len(chanGraph.Edges) } - aliceChans := numChannels(net.Alice) - if aliceChans != 4 { - t.Fatalf("expected Alice to know 4 edges, had %v", aliceChans) - } - bobChans := numChannels(net.Bob) - if bobChans != 3 { - t.Fatalf("expected Bob to know 3 edges, had %v", bobChans) - } - carolChans := numChannels(carol) - if carolChans != 4 { - t.Fatalf("expected Carol to know 4 edges, had %v", carolChans) - } - daveChans := numChannels(dave) - if daveChans != 3 { - t.Fatalf("expected Dave to know 3 edges, had %v", daveChans) + var predErr error + err = lntest.WaitPredicate(func() bool { + aliceChans := numChannels(net.Alice) + if aliceChans != 4 { + predErr = fmt.Errorf("expected Alice to know 4 edges, "+ + "had %v", aliceChans) + return false + } + bobChans := numChannels(net.Bob) + if bobChans != 3 { + predErr = fmt.Errorf("expected Bob to know 3 edges, "+ + "had %v", bobChans) + return false + } + carolChans := numChannels(carol) + if carolChans != 4 { + predErr = fmt.Errorf("expected Carol to know 4 edges, "+ + "had %v", carolChans) + return false + } + daveChans := numChannels(dave) + if daveChans != 3 { + predErr = fmt.Errorf("expected Dave to know 3 edges, "+ + "had %v", daveChans) + return false + } + return true + }, time.Second*15) + if err != nil { + t.Fatalf("%v", predErr) } // Close all channels.