From 1f781ea4317173b523ccf629252eb4c04699bd09 Mon Sep 17 00:00:00 2001 From: Wilmer Paulino Date: Fri, 13 Dec 2019 16:07:53 -0800 Subject: [PATCH 1/6] discovery: use inclusive range in FilterChannelRange FilterChannelRange takes an inclusive range, so it was possible for us to return channels for an additional block that was not requested. --- discovery/syncer.go | 3 ++- discovery/syncer_test.go | 13 +++++++++---- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/discovery/syncer.go b/discovery/syncer.go index 9edb02beb..b7dd07f85 100644 --- a/discovery/syncer.go +++ b/discovery/syncer.go @@ -814,8 +814,9 @@ func (g *GossipSyncer) replyChanRangeQuery(query *lnwire.QueryChannelRange) erro // Next, we'll consult the time series to obtain the set of known // channel ID's that match their query. startBlock := query.FirstBlockHeight + endBlock := startBlock + query.NumBlocks - 1 channelRange, err := g.cfg.channelSeries.FilterChannelRange( - query.ChainHash, startBlock, startBlock+query.NumBlocks, + query.ChainHash, startBlock, endBlock, ) if err != nil { return err diff --git a/discovery/syncer_test.go b/discovery/syncer_test.go index 60ba3d5fe..9ca2f331b 100644 --- a/discovery/syncer_test.go +++ b/discovery/syncer_test.go @@ -709,10 +709,12 @@ func TestGossipSyncerReplyChanRangeQuery(t *testing.T) { // Next, we'll craft a query to ask for all the new chan ID's after // block 100. - const startingBlockHeight int = 100 + const startingBlockHeight = 100 + const numBlocks = 50 + const endingBlockHeight = startingBlockHeight + numBlocks - 1 query := &lnwire.QueryChannelRange{ FirstBlockHeight: uint32(startingBlockHeight), - NumBlocks: 50, + NumBlocks: uint32(numBlocks), } // We'll then launch a goroutine to reply to the query with a set of 5 @@ -744,8 +746,11 @@ func TestGossipSyncerReplyChanRangeQuery(t *testing.T) { return case filterReq := <-chanSeries.filterRangeReqs: // We should be querying for block 100 to 150. - if filterReq.startHeight != 100 && filterReq.endHeight != 150 { - errCh <- fmt.Errorf("wrong height range: %v", spew.Sdump(filterReq)) + if filterReq.startHeight != startingBlockHeight && + filterReq.endHeight != endingBlockHeight { + + errCh <- fmt.Errorf("wrong height range: %v", + spew.Sdump(filterReq)) return } From c7c0853531d4fa0aa818e617b6253efa6df3b602 Mon Sep 17 00:00:00 2001 From: Wilmer Paulino Date: Fri, 13 Dec 2019 16:08:10 -0800 Subject: [PATCH 2/6] discovery: cover requested range in ReplyChannelRange messages In order to properly adhere to the spec, when handling a QueryChannelRange message, we must reply with a series of ReplyChannelRange messages, that when consumed together cover the entirety of the block range requested. --- discovery/syncer.go | 35 ++++++++++++----- discovery/syncer_test.go | 81 +++++++++++++++++++++------------------- 2 files changed, 68 insertions(+), 48 deletions(-) diff --git a/discovery/syncer.go b/discovery/syncer.go index b7dd07f85..9d24e50cc 100644 --- a/discovery/syncer.go +++ b/discovery/syncer.go @@ -825,13 +825,13 @@ func (g *GossipSyncer) replyChanRangeQuery(query *lnwire.QueryChannelRange) erro // TODO(roasbeef): means can't send max uint above? // * or make internal 64 - // In the base case (no actual response) the first block and the last - // block in the query will be the same. In the loop below, we'll update - // these two variables incrementally with each chunk to properly - // compute the starting block for each response and the number of - // blocks in a response. - firstBlockHeight := query.FirstBlockHeight - lastBlockHeight := query.FirstBlockHeight + // In the base case (no actual response) the first block and last block + // will match those of the query. In the loop below, we'll update these + // two variables incrementally with each chunk to properly compute the + // starting block for each response and the number of blocks in a + // response. + firstBlockHeight := startBlock + lastBlockHeight := endBlock numChannels := int32(len(channelRange)) numChansSent := int32(0) @@ -867,8 +867,25 @@ func (g *GossipSyncer) replyChanRangeQuery(query *lnwire.QueryChannelRange) erro // update our pointers to the first and last blocks for each // response. if len(channelChunk) > 0 { - firstBlockHeight = channelChunk[0].BlockHeight - lastBlockHeight = channelChunk[len(channelChunk)-1].BlockHeight + // If this is the first response we'll send, we'll point + // the first block to the first block in the query. + // Otherwise, we'll continue from the block we left off + // at. + if numChansSent == 0 { + firstBlockHeight = startBlock + } else { + firstBlockHeight = lastBlockHeight + } + + // If this is the last response we'll send, we'll point + // the last block to the last block of the query. + // Otherwise, we'll set it to the height of the last + // channel in the chunk. + if isFinalChunk { + lastBlockHeight = endBlock + } else { + lastBlockHeight = channelChunk[len(channelChunk)-1].BlockHeight + } } // The number of blocks contained in this response (the total diff --git a/discovery/syncer_test.go b/discovery/syncer_test.go index 9ca2f331b..ee1d880a1 100644 --- a/discovery/syncer_test.go +++ b/discovery/syncer_test.go @@ -783,52 +783,55 @@ func TestGossipSyncerReplyChanRangeQuery(t *testing.T) { t.Fatalf("expected ReplyChannelRange instead got %T", msg) } - // Only for the first iteration do we set the offset to - // zero as no chunks have been processed yet. For every - // other iteration, we want to move forward by the - // chunkSize (from the staring block height). - offset := 0 - if i != 0 { - offset = 1 - } - expectedFirstBlockHeight := (i+offset)*2 + startingBlockHeight + // We'll determine the correct values of each field in + // each response based on the order that they were sent. + var ( + expectedFirstBlockHeight uint32 + expectedNumBlocks uint32 + expectedComplete uint8 + ) switch { - // If this is not the last chunk, then Complete should - // be set to zero. Otherwise, it should be one. - case i < 2 && rangeResp.Complete != 0: - t.Fatalf("non-final chunk should have "+ - "Complete=0: %v", spew.Sdump(rangeResp)) + // The first reply should range from our starting block + // height until it reaches its maximum capacity of + // channels. + case i == 0: + expectedFirstBlockHeight = startingBlockHeight + expectedNumBlocks = chunkSize + 1 - case i < 2 && rangeResp.NumBlocks != chunkSize+1: - t.Fatalf("NumBlocks fields in resp "+ - "incorrect: expected %v got %v", - chunkSize+1, rangeResp.NumBlocks) + // The last reply should range starting from the next + // block of our previous reply up until the ending + // height of the query. It should also have the Complete + // bit set. + case i == numExpectedChunks-1: + expectedFirstBlockHeight = respMsgs[len(respMsgs)-1].BlockHeight + expectedNumBlocks = endingBlockHeight - expectedFirstBlockHeight + 1 + expectedComplete = 1 - case i < 2 && rangeResp.FirstBlockHeight != - uint32(expectedFirstBlockHeight): + // Any intermediate replies should range starting from + // the next block of our previous reply up until it + // reaches its maximum capacity of channels. + default: + expectedFirstBlockHeight = respMsgs[len(respMsgs)-1].BlockHeight + expectedNumBlocks = 5 + } - t.Fatalf("FirstBlockHeight incorrect: "+ - "expected %v got %v", - rangeResp.FirstBlockHeight, - expectedFirstBlockHeight) - case i == 2 && rangeResp.Complete != 1: - t.Fatalf("final chunk should have "+ - "Complete=1: %v", spew.Sdump(rangeResp)) + switch { + case rangeResp.FirstBlockHeight != expectedFirstBlockHeight: + t.Fatalf("FirstBlockHeight in resp #%d "+ + "incorrect: expected %v, got %v", i+1, + expectedFirstBlockHeight, + rangeResp.FirstBlockHeight) - case i == 2 && rangeResp.NumBlocks != 1: - t.Fatalf("NumBlocks fields in resp "+ - "incorrect: expected %v got %v", 1, - rangeResp.NumBlocks) - - case i == 2 && rangeResp.FirstBlockHeight != - queryResp[len(queryResp)-1].BlockHeight: - - t.Fatalf("FirstBlockHeight incorrect: "+ - "expected %v got %v", - rangeResp.FirstBlockHeight, - queryResp[len(queryResp)-1].BlockHeight) + case rangeResp.NumBlocks != expectedNumBlocks: + t.Fatalf("NumBlocks in resp #%d incorrect: "+ + "expected %v, got %v", i+1, + expectedNumBlocks, rangeResp.NumBlocks) + case rangeResp.Complete != expectedComplete: + t.Fatalf("Complete in resp #%d incorrect: "+ + "expected %v, got %v", i+1, + expectedNumBlocks, rangeResp.Complete) } respMsgs = append(respMsgs, rangeResp.ShortChanIDs...) From c4723ce3db715602596fc1af87f935a49dcea2b1 Mon Sep 17 00:00:00 2001 From: Wilmer Paulino Date: Fri, 13 Dec 2019 16:08:30 -0800 Subject: [PATCH 3/6] lnwire: add LastBlockHeight method to QueryChannelRange --- lnwire/query_channel_range.go | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/lnwire/query_channel_range.go b/lnwire/query_channel_range.go index f910cb988..9546fcd32 100644 --- a/lnwire/query_channel_range.go +++ b/lnwire/query_channel_range.go @@ -2,6 +2,7 @@ package lnwire import ( "io" + "math" "github.com/btcsuite/btcd/chaincfg/chainhash" ) @@ -75,3 +76,14 @@ func (q *QueryChannelRange) MaxPayloadLength(uint32) uint32 { // 32 + 4 + 4 return 40 } + +// LastBlockHeight returns the last block height covered by the range of a +// QueryChannelRange message. +func (q *QueryChannelRange) LastBlockHeight() uint32 { + // Handle overflows by casting to uint64. + lastBlockHeight := uint64(q.FirstBlockHeight) + uint64(q.NumBlocks) - 1 + if lastBlockHeight > math.MaxUint32 { + return math.MaxUint32 + } + return uint32(lastBlockHeight) +} From d688e13d356a4e8b5dfe3e8d9aeeb79eef04cdba Mon Sep 17 00:00:00 2001 From: Wilmer Paulino Date: Fri, 13 Dec 2019 16:08:43 -0800 Subject: [PATCH 4/6] discovery: remove unnecessary test check It's not possible to send another reply once all replies have been sent without another request. The purpose of the check is also done within another test, TestGossipSyncerReplyChanRangeQueryNoNewChans, so it can be removed from here. --- discovery/syncer_test.go | 46 +--------------------------------------- 1 file changed, 1 insertion(+), 45 deletions(-) diff --git a/discovery/syncer_test.go b/discovery/syncer_test.go index ee1d880a1..82c07044f 100644 --- a/discovery/syncer_test.go +++ b/discovery/syncer_test.go @@ -989,9 +989,7 @@ func TestGossipSyncerGenChanRangeQuery(t *testing.T) { } // TestGossipSyncerProcessChanRangeReply tests that we'll properly buffer -// replied channel replies until we have the complete version. If no new -// channels were discovered, then we should go directly to the chanSsSynced -// state. Otherwise, we should go to the queryNewChannels states. +// replied channel replies until we have the complete version. func TestGossipSyncerProcessChanRangeReply(t *testing.T) { t.Parallel() @@ -1091,48 +1089,6 @@ func TestGossipSyncerProcessChanRangeReply(t *testing.T) { t.Fatal(err) } } - - // We'll repeat our final reply again, but this time we won't send any - // new channels. As a result, we should transition over to the - // chansSynced state. - errCh = make(chan error, 1) - go func() { - select { - case <-time.After(time.Second * 15): - errCh <- errors.New("no query received") - return - case req := <-chanSeries.filterReq: - // We should get a request for the entire range of short - // chan ID's. - if !reflect.DeepEqual(expectedReq[2], req[0]) { - errCh <- fmt.Errorf("wrong request: expected %v, got %v", - expectedReq[2], req[0]) - return - } - - // We'll send back only the last two to simulate filtering. - chanSeries.filterResp <- []lnwire.ShortChannelID{} - errCh <- nil - } - }() - if err := syncer.processChanRangeReply(replies[2]); err != nil { - t.Fatalf("unable to process reply: %v", err) - } - - if syncer.syncState() != chansSynced { - t.Fatalf("wrong state: expected %v instead got %v", - chansSynced, syncer.state) - } - - // Wait for error from goroutine. - select { - case <-time.After(time.Second * 30): - t.Fatalf("goroutine did not return within 30 seconds") - case err := <-errCh: - if err != nil { - t.Fatal(err) - } - } } // TestGossipSyncerSynchronizeChanIDs tests that we properly request chunks of From 1bacdfb41e3f97ffcac56501c9674023f69c08ab Mon Sep 17 00:00:00 2001 From: Wilmer Paulino Date: Fri, 13 Dec 2019 16:08:58 -0800 Subject: [PATCH 5/6] discovery: interpret block range from ReplyChannelRange messages We move from our legacy way of interpreting ReplyChannelRange messages which was incorrect. Previously, we'd rely on the Complete field of the ReplyChannelRange message to determine when our peer had sent all of their replies. Now, we properly adhere to the specification by interpreting the block ranges of these messages as intended. Due to the large number of nodes deployed with the previous method, we still maintain and detect when we are communicating with them, such that we are still able to sync with them for backwards compatibility. --- discovery/sync_manager_test.go | 10 ++- discovery/syncer.go | 99 +++++++++++++++++++++++-- discovery/syncer_test.go | 128 ++++++++++++++++++++++++--------- 3 files changed, 196 insertions(+), 41 deletions(-) diff --git a/discovery/sync_manager_test.go b/discovery/sync_manager_test.go index d165447f5..c7a228f8c 100644 --- a/discovery/sync_manager_test.go +++ b/discovery/sync_manager_test.go @@ -529,12 +529,16 @@ func assertSyncerStatus(t *testing.T, s *GossipSyncer, syncState syncerState, func assertTransitionToChansSynced(t *testing.T, s *GossipSyncer, peer *mockPeer) { t.Helper() - assertMsgSent(t, peer, &lnwire.QueryChannelRange{ + query := &lnwire.QueryChannelRange{ FirstBlockHeight: 0, NumBlocks: math.MaxUint32, - }) + } + assertMsgSent(t, peer, query) - s.ProcessQueryMsg(&lnwire.ReplyChannelRange{Complete: 1}, nil) + s.ProcessQueryMsg(&lnwire.ReplyChannelRange{ + QueryChannelRange: *query, + Complete: 1, + }, nil) chanSeries := s.cfg.channelSeries.(*mockChannelGraphTimeSeries) diff --git a/discovery/syncer.go b/discovery/syncer.go index 9d24e50cc..8348abc56 100644 --- a/discovery/syncer.go +++ b/discovery/syncer.go @@ -298,6 +298,17 @@ type GossipSyncer struct { // received over, these will be read by the replyHandler. queryMsgs chan lnwire.Message + // curQueryRangeMsg keeps track of the latest QueryChannelRange message + // we've sent to a peer to ensure we've consumed all expected replies. + // This field is primarily used within the waitingQueryChanReply state. + curQueryRangeMsg *lnwire.QueryChannelRange + + // prevReplyChannelRange keeps track of the previous ReplyChannelRange + // message we've received from a peer to ensure they've fully replied to + // our query by ensuring they covered our requested block range. This + // field is primarily used within the waitingQueryChanReply state. + prevReplyChannelRange *lnwire.ReplyChannelRange + // bufferedChanRangeReplies is used in the waitingQueryChanReply to // buffer all the chunked response to our query. bufferedChanRangeReplies []lnwire.ShortChannelID @@ -666,10 +677,64 @@ func (g *GossipSyncer) synchronizeChanIDs() (bool, error) { return false, err } +// isLegacyReplyChannelRange determines where a ReplyChannelRange message is +// considered legacy. There was a point where lnd used to include the same query +// over multiple replies, rather than including the portion of the query the +// reply is handling. We'll use this as a way of detecting whether we are +// communicating with a legacy node so we can properly sync with them. +func isLegacyReplyChannelRange(query *lnwire.QueryChannelRange, + reply *lnwire.ReplyChannelRange) bool { + + return reply.QueryChannelRange == *query +} + // processChanRangeReply is called each time the GossipSyncer receives a new // reply to the initial range query to discover new channels that it didn't // previously know of. func (g *GossipSyncer) processChanRangeReply(msg *lnwire.ReplyChannelRange) error { + // If we're not communicating with a legacy node, we'll apply some + // further constraints on their reply to ensure it satisfies our query. + if !isLegacyReplyChannelRange(g.curQueryRangeMsg, msg) { + // The first block should be within our original request. + if msg.FirstBlockHeight < g.curQueryRangeMsg.FirstBlockHeight { + return fmt.Errorf("reply includes channels for height "+ + "%v prior to query %v", msg.FirstBlockHeight, + g.curQueryRangeMsg.FirstBlockHeight) + } + + // The last block should also be. We don't need to check the + // intermediate ones because they should already be in sorted + // order. + replyLastHeight := msg.QueryChannelRange.LastBlockHeight() + queryLastHeight := g.curQueryRangeMsg.LastBlockHeight() + if replyLastHeight > queryLastHeight { + return fmt.Errorf("reply includes channels for height "+ + "%v after query %v", replyLastHeight, + queryLastHeight) + } + + // If we've previously received a reply for this query, look at + // its last block to ensure the current reply properly follows + // it. + if g.prevReplyChannelRange != nil { + prevReply := g.prevReplyChannelRange + prevReplyLastHeight := prevReply.LastBlockHeight() + + // The current reply can either start from the previous + // reply's last block, if there are still more channels + // for the same block, or the block after. + if msg.FirstBlockHeight != prevReplyLastHeight && + msg.FirstBlockHeight != prevReplyLastHeight+1 { + + return fmt.Errorf("first block of reply %v "+ + "does not continue from last block of "+ + "previous %v", msg.FirstBlockHeight, + prevReplyLastHeight) + } + } + } + + g.prevReplyChannelRange = msg g.bufferedChanRangeReplies = append( g.bufferedChanRangeReplies, msg.ShortChanIDs..., ) @@ -679,8 +744,25 @@ func (g *GossipSyncer) processChanRangeReply(msg *lnwire.ReplyChannelRange) erro // If this isn't the last response, then we can exit as we've already // buffered the latest portion of the streaming reply. - if msg.Complete == 0 { - return nil + switch { + // If we're communicating with a legacy node, we'll need to look at the + // complete field. + case isLegacyReplyChannelRange(g.curQueryRangeMsg, msg): + if msg.Complete == 0 { + return nil + } + + // Otherwise, we'll look at the reply's height range. + default: + replyLastHeight := msg.QueryChannelRange.LastBlockHeight() + queryLastHeight := g.curQueryRangeMsg.LastBlockHeight() + + // TODO(wilmer): This might require some padding if the remote + // node is not aware of the last height we sent them, i.e., is + // behind a few blocks from us. + if replyLastHeight < queryLastHeight { + return nil + } } log.Infof("GossipSyncer(%x): filtering through %v chans", @@ -696,8 +778,10 @@ func (g *GossipSyncer) processChanRangeReply(msg *lnwire.ReplyChannelRange) erro } // As we've received the entirety of the reply, we no longer need to - // hold on to the set of buffered replies, so we'll let that be garbage - // collected now. + // hold on to the set of buffered replies or the original query that + // prompted the replies, so we'll let that be garbage collected now. + g.curQueryRangeMsg = nil + g.prevReplyChannelRange = nil g.bufferedChanRangeReplies = nil // If there aren't any channels that we don't know of, then we can @@ -757,11 +841,14 @@ func (g *GossipSyncer) genChanRangeQuery( // Finally, we'll craft the channel range query, using our starting // height, then asking for all known channels to the foreseeable end of // the main chain. - return &lnwire.QueryChannelRange{ + query := &lnwire.QueryChannelRange{ ChainHash: g.cfg.chainHash, FirstBlockHeight: startHeight, NumBlocks: math.MaxUint32 - startHeight, - }, nil + } + g.curQueryRangeMsg = query + + return query, nil } // replyPeerQueries is called in response to any query by the remote peer. diff --git a/discovery/syncer_test.go b/discovery/syncer_test.go index 82c07044f..1b1f88909 100644 --- a/discovery/syncer_test.go +++ b/discovery/syncer_test.go @@ -993,29 +993,87 @@ func TestGossipSyncerGenChanRangeQuery(t *testing.T) { func TestGossipSyncerProcessChanRangeReply(t *testing.T) { t.Parallel() + t.Run("legacy", func(t *testing.T) { + testGossipSyncerProcessChanRangeReply(t, true) + }) + t.Run("block ranges", func(t *testing.T) { + testGossipSyncerProcessChanRangeReply(t, false) + }) +} + +// testGossipSyncerProcessChanRangeReply tests that we'll properly buffer +// replied channel replies until we have the complete version. The legacy +// option, if set, uses the Complete field of the reply to determine when we've +// received all expected replies. Otherwise, it looks at the block ranges of +// each reply instead. +func testGossipSyncerProcessChanRangeReply(t *testing.T, legacy bool) { + t.Parallel() + // First, we'll create a GossipSyncer instance with a canned sendToPeer // message to allow us to intercept their potential sends. + highestID := lnwire.ShortChannelID{ + BlockHeight: latestKnownHeight, + } _, syncer, chanSeries := newTestSyncer( - lnwire.NewShortChanIDFromInt(10), defaultEncoding, defaultChunkSize, + highestID, defaultEncoding, defaultChunkSize, ) startingState := syncer.state + query, err := syncer.genChanRangeQuery(true) + if err != nil { + t.Fatalf("unable to generate channel range query: %v", err) + } + + var replyQueries []*lnwire.QueryChannelRange + if legacy { + // Each reply query is the same as the original query in the + // legacy mode. + replyQueries = []*lnwire.QueryChannelRange{query, query, query} + } else { + // When interpreting block ranges, the first reply should start + // from our requested first block, and the last should end at + // our requested last block. + replyQueries = []*lnwire.QueryChannelRange{ + { + FirstBlockHeight: 0, + NumBlocks: 11, + }, + { + FirstBlockHeight: 11, + NumBlocks: 1, + }, + { + FirstBlockHeight: 12, + NumBlocks: query.NumBlocks - 12, + }, + } + } + replies := []*lnwire.ReplyChannelRange{ { + QueryChannelRange: *replyQueries[0], ShortChanIDs: []lnwire.ShortChannelID{ - lnwire.NewShortChanIDFromInt(10), + { + BlockHeight: 10, + }, }, }, { + QueryChannelRange: *replyQueries[1], ShortChanIDs: []lnwire.ShortChannelID{ - lnwire.NewShortChanIDFromInt(11), + { + BlockHeight: 11, + }, }, }, { - Complete: 1, + QueryChannelRange: *replyQueries[2], + Complete: 1, ShortChanIDs: []lnwire.ShortChannelID{ - lnwire.NewShortChanIDFromInt(12), + { + BlockHeight: 12, + }, }, }, } @@ -1036,9 +1094,15 @@ func TestGossipSyncerProcessChanRangeReply(t *testing.T) { } expectedReq := []lnwire.ShortChannelID{ - lnwire.NewShortChanIDFromInt(10), - lnwire.NewShortChanIDFromInt(11), - lnwire.NewShortChanIDFromInt(12), + { + BlockHeight: 10, + }, + { + BlockHeight: 11, + }, + { + BlockHeight: 12, + }, } // As we're about to send the final response, we'll launch a goroutine @@ -1224,17 +1288,17 @@ func TestGossipSyncerDelayDOS(t *testing.T) { // First, we'll create two GossipSyncer instances with a canned // sendToPeer message to allow us to intercept their potential sends. - startHeight := lnwire.ShortChannelID{ + highestID := lnwire.ShortChannelID{ BlockHeight: 1144, } msgChan1, syncer1, chanSeries1 := newTestSyncer( - startHeight, defaultEncoding, chunkSize, true, false, + highestID, defaultEncoding, chunkSize, true, false, ) syncer1.Start() defer syncer1.Stop() msgChan2, syncer2, chanSeries2 := newTestSyncer( - startHeight, defaultEncoding, chunkSize, false, true, + highestID, defaultEncoding, chunkSize, false, true, ) syncer2.Start() defer syncer2.Stop() @@ -1264,9 +1328,10 @@ func TestGossipSyncerDelayDOS(t *testing.T) { // inherently disjoint. var syncer2Chans []lnwire.ShortChannelID for i := 0; i < numTotalChans; i++ { - syncer2Chans = append( - syncer2Chans, lnwire.NewShortChanIDFromInt(uint64(i)), - ) + syncer2Chans = append(syncer2Chans, lnwire.ShortChannelID{ + BlockHeight: highestID.BlockHeight - 1, + TxIndex: uint32(i), + }) } // We'll kick off the test by asserting syncer1 sends over the @@ -1494,17 +1559,17 @@ func TestGossipSyncerRoutineSync(t *testing.T) { // First, we'll create two GossipSyncer instances with a canned // sendToPeer message to allow us to intercept their potential sends. - startHeight := lnwire.ShortChannelID{ + highestID := lnwire.ShortChannelID{ BlockHeight: 1144, } msgChan1, syncer1, chanSeries1 := newTestSyncer( - startHeight, defaultEncoding, chunkSize, true, false, + highestID, defaultEncoding, chunkSize, true, false, ) syncer1.Start() defer syncer1.Stop() msgChan2, syncer2, chanSeries2 := newTestSyncer( - startHeight, defaultEncoding, chunkSize, false, true, + highestID, defaultEncoding, chunkSize, false, true, ) syncer2.Start() defer syncer2.Stop() @@ -1512,9 +1577,9 @@ func TestGossipSyncerRoutineSync(t *testing.T) { // Although both nodes are at the same height, syncer will have 3 chan // ID's that syncer1 doesn't know of. syncer2Chans := []lnwire.ShortChannelID{ - lnwire.NewShortChanIDFromInt(4), - lnwire.NewShortChanIDFromInt(5), - lnwire.NewShortChanIDFromInt(6), + {BlockHeight: highestID.BlockHeight - 3}, + {BlockHeight: highestID.BlockHeight - 2}, + {BlockHeight: highestID.BlockHeight - 1}, } // We'll kick off the test by passing over the QueryChannelRange @@ -1638,35 +1703,34 @@ func TestGossipSyncerAlreadySynced(t *testing.T) { // our chunk parsing works properly. With this value we should get 3 // queries: two full chunks, and one lingering chunk. const chunkSize = 2 + const numChans = 3 // First, we'll create two GossipSyncer instances with a canned // sendToPeer message to allow us to intercept their potential sends. - startHeight := lnwire.ShortChannelID{ + highestID := lnwire.ShortChannelID{ BlockHeight: 1144, } msgChan1, syncer1, chanSeries1 := newTestSyncer( - startHeight, defaultEncoding, chunkSize, + highestID, defaultEncoding, chunkSize, ) syncer1.Start() defer syncer1.Stop() msgChan2, syncer2, chanSeries2 := newTestSyncer( - startHeight, defaultEncoding, chunkSize, + highestID, defaultEncoding, chunkSize, ) syncer2.Start() defer syncer2.Stop() // The channel state of both syncers will be identical. They should // recognize this, and skip the sync phase below. - syncer1Chans := []lnwire.ShortChannelID{ - lnwire.NewShortChanIDFromInt(1), - lnwire.NewShortChanIDFromInt(2), - lnwire.NewShortChanIDFromInt(3), - } - syncer2Chans := []lnwire.ShortChannelID{ - lnwire.NewShortChanIDFromInt(1), - lnwire.NewShortChanIDFromInt(2), - lnwire.NewShortChanIDFromInt(3), + var syncer1Chans, syncer2Chans []lnwire.ShortChannelID + for i := numChans; i > 0; i-- { + shortChanID := lnwire.ShortChannelID{ + BlockHeight: highestID.BlockHeight - uint32(i), + } + syncer1Chans = append(syncer1Chans, shortChanID) + syncer2Chans = append(syncer2Chans, shortChanID) } // We'll now kick off the test by allowing both side to send their From 9a7f66f2f73f97bceb74b5b23246bf4f6c6667fa Mon Sep 17 00:00:00 2001 From: Wilmer Paulino Date: Fri, 13 Dec 2019 16:09:16 -0800 Subject: [PATCH 6/6] peer: modify ReplyChannelRange log message The message now shows the block range the reply spans, which is a lot more useful. --- peer.go | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/peer.go b/peer.go index 53092edbd..7a9014135 100644 --- a/peer.go +++ b/peer.go @@ -1336,8 +1336,10 @@ func messageSummary(msg lnwire.Message) string { msg.Complete) case *lnwire.ReplyChannelRange: - return fmt.Sprintf("complete=%v, encoding=%v, num_chans=%v", - msg.Complete, msg.EncodingType, len(msg.ShortChanIDs)) + return fmt.Sprintf("start_height=%v, end_height=%v, "+ + "num_chans=%v, encoding=%v", msg.FirstBlockHeight, + msg.LastBlockHeight(), len(msg.ShortChanIDs), + msg.EncodingType) case *lnwire.QueryShortChanIDs: return fmt.Sprintf("chain_hash=%v, encoding=%v, num_chans=%v", @@ -1345,8 +1347,8 @@ func messageSummary(msg lnwire.Message) string { case *lnwire.QueryChannelRange: return fmt.Sprintf("chain_hash=%v, start_height=%v, "+ - "num_blocks=%v", msg.ChainHash, msg.FirstBlockHeight, - msg.NumBlocks) + "end_height=%v", msg.ChainHash, msg.FirstBlockHeight, + msg.LastBlockHeight()) case *lnwire.GossipTimestampRange: return fmt.Sprintf("chain_hash=%v, first_stamp=%v, "+