cmd/devp2p/v5test: fix FindnodeResults bystanders

cmd/devp2p/internal/v5test/discv5tests.go

@@ -295,21 +295,33 @@ that they are returned by FINDNODE.`)
 	t.Log("requesting nodes")
 	conn, l1 := s.listen1(t)
 	defer conn.close()
-	foundNodes, err := conn.findnode(l1, dists)
+
-	if err != nil {
+	const maxAttempts = 15
-		t.Fatal(err)
+	const retryInterval = 3 * time.Second
-	}
+
-	t.Logf("remote returned %d nodes for distance list %v", len(foundNodes), dists)
+	for attempt := 1; attempt <= maxAttempts; attempt++ {
-	for _, n := range foundNodes {
+		foundNodes, err := conn.findnode(l1, dists)
-		delete(expect, n.ID())
+		if err != nil {
-	}
+			t.Fatal(err)
-	if len(expect) > 0 {
+		}
-		t.Errorf("missing %d nodes in FINDNODE result", len(expect))
+		missing := make(map[enode.ID]struct{})
-		t.Logf("this can happen if the test is run multiple times in quick succession")
+		for id := range expect {
-		t.Logf("and the remote node hasn't removed dead nodes from previous runs yet")
+			missing[id] = struct{}{}
-	} else {
+		}
-		t.Logf("all %d expected nodes were returned", len(nodes))
+		for _, n := range foundNodes {
+			delete(missing, n.ID())
+		}
+		t.Logf("attempt %d: remote returned %d nodes for distance list %v, missing %d", attempt, len(foundNodes), dists, len(missing))
+		if len(missing) == 0 {
+			t.Logf("all %d expected nodes were returned", len(nodes))
+			return
+		}
+		if attempt < maxAttempts {
+			time.Sleep(retryInterval)
+		}
 	}
+	t.Errorf("missing nodes in FINDNODE result after %d attempts", maxAttempts)
+	t.Logf("this can happen if the node has a non-empty table from previous runs")
 }
 
 // A bystander is a node whose only purpose is filling a spot in the remote table.
@@ -331,6 +343,12 @@ func newBystander(t *utesting.T, s *Suite, added chan enode.ID) *bystander {
 		dest:    s.Dest,
 		addedCh: added,
 	}
+	// Establish an initial session and let the remote learn this node before
+	// switching to the passive responder loop below.
+	conn.reqresp(l, &v5wire.Ping{
+		ReqID:  conn.nextReqID(),
+		ENRSeq: s.Dest.Seq(),
+	})
 	bn.done.Add(1)
 	go bn.loop()
 	return bn
@@ -351,21 +369,14 @@ func (bn *bystander) close() {
 func (bn *bystander) loop() {
 	defer bn.done.Done()
 
-	var (
-		lastPing time.Time
-		wasAdded bool
-	)
 	for {
-		// Ping the remote node.
+		switch p := bn.conn.read(bn.l).(type) {
-		if !wasAdded && time.Since(lastPing) > 10*time.Second {
+		case *v5wire.Whoareyou:
-			bn.conn.reqresp(bn.l, &v5wire.Ping{
+			p.Node = bn.dest
+			bn.conn.write(bn.l, &v5wire.Ping{
 				ReqID:  bn.conn.nextReqID(),
 				ENRSeq: bn.dest.Seq(),
-			})
+			}, p)
-			lastPing = time.Now()
-		}
-		// Answer packets.
-		switch p := bn.conn.read(bn.l).(type) {
 		case *v5wire.Ping:
 			bn.conn.write(bn.l, &v5wire.Pong{
 				ReqID:  p.ReqID,
@@ -373,19 +384,18 @@ func (bn *bystander) loop() {
 				ToIP:   bn.dest.IP(),
 				ToPort: uint16(bn.dest.UDP()),
 			}, nil)
-			wasAdded = true
 			bn.notifyAdded()
 		case *v5wire.Findnode:
 			bn.conn.write(bn.l, &v5wire.Nodes{ReqID: p.ReqID, RespCount: 1}, nil)
-			wasAdded = true
 			bn.notifyAdded()
 		case *v5wire.TalkRequest:
 			bn.conn.write(bn.l, &v5wire.TalkResponse{ReqID: p.ReqID}, nil)
 		case *readError:
-			if !netutil.IsTemporaryError(p.err) {
+			if netutil.IsTemporaryError(p.err) || v5wire.IsInvalidHeader(p.err) {
-				bn.conn.logf("shutting down: %v", p.err)
+				continue
-				return
 			}
+			bn.conn.logf("shutting down: %v", p.err)
+			return
 		}
 	}
 }