diff --git a/p2p/discover/node_test.go b/p2p/discover/node_test.go index b1babd9892..795460c498 100644 --- a/p2p/discover/node_test.go +++ b/p2p/discover/node_test.go @@ -13,11 +13,6 @@ import ( "github.com/ethereum/go-ethereum/crypto" ) -var ( - quickrand = rand.New(rand.NewSource(time.Now().Unix())) - quickcfg = &quick.Config{MaxCount: 5000, Rand: quickrand} -) - var parseNodeTests = []struct { rawurl string wantError string @@ -176,7 +171,7 @@ func TestNodeID_distcmp(t *testing.T) { bbig := new(big.Int).SetBytes(b[:]) return new(big.Int).Xor(tbig, abig).Cmp(new(big.Int).Xor(tbig, bbig)) } - if err := quick.CheckEqual(distcmp, distcmpBig, quickcfg); err != nil { + if err := quick.CheckEqual(distcmp, distcmpBig, quickcfg()); err != nil { t.Error(err) } } @@ -195,7 +190,7 @@ func TestNodeID_logdist(t *testing.T) { abig, bbig := new(big.Int).SetBytes(a[:]), new(big.Int).SetBytes(b[:]) return new(big.Int).Xor(abig, bbig).BitLen() } - if err := quick.CheckEqual(logdist, logdistBig, quickcfg); err != nil { + if err := quick.CheckEqual(logdist, logdistBig, quickcfg()); err != nil { t.Error(err) } } @@ -211,9 +206,10 @@ func TestNodeID_logdistEqual(t *testing.T) { func TestNodeID_hashAtDistance(t *testing.T) { // we don't use quick.Check here because its output isn't // very helpful when the test fails. - for i := 0; i < quickcfg.MaxCount; i++ { - a := gen(common.Hash{}, quickrand).(common.Hash) - dist := quickrand.Intn(len(common.Hash{}) * 8) + cfg := quickcfg() + for i := 0; i < cfg.MaxCount; i++ { + a := gen(common.Hash{}, cfg.Rand).(common.Hash) + dist := cfg.Rand.Intn(len(common.Hash{}) * 8) result := hashAtDistance(a, dist) actualdist := logdist(result, a) @@ -225,7 +221,14 @@ func TestNodeID_hashAtDistance(t *testing.T) { } } -// TODO: this can be dropped when we require Go >= 1.5 +func quickcfg() *quick.Config { + return &quick.Config{ + MaxCount: 5000, + Rand: rand.New(rand.NewSource(time.Now().Unix())), + } +} + +// TODO: The Generate method can be dropped when we require Go >= 1.5 // because testing/quick learned to generate arrays in 1.5. func (NodeID) Generate(rand *rand.Rand, size int) reflect.Value { diff --git a/p2p/discover/table.go b/p2p/discover/table.go index 4b7ddb7759..f713204258 100644 --- a/p2p/discover/table.go +++ b/p2p/discover/table.go @@ -40,6 +40,8 @@ type Table struct { bonding map[NodeID]*bondproc bondslots chan struct{} // limits total number of active bonding processes + nodeAddedHook func(*Node) // for testing + net transport self *Node // metadata of the local node } @@ -431,6 +433,9 @@ func (tab *Table) pingreplace(new *Node, b *bucket) { } copy(b.entries[1:], b.entries) b.entries[0] = new + if tab.nodeAddedHook != nil { + tab.nodeAddedHook(new) + } } // ping a remote endpoint and wait for a reply, also updating the node database @@ -466,6 +471,9 @@ outer: } if len(bucket.entries) < bucketSize { bucket.entries = append(bucket.entries, n) + if tab.nodeAddedHook != nil { + tab.nodeAddedHook(n) + } } } } diff --git a/p2p/discover/table_test.go b/p2p/discover/table_test.go index da398d1373..8298999161 100644 --- a/p2p/discover/table_test.go +++ b/p2p/discover/table_test.go @@ -9,6 +9,7 @@ import ( "reflect" "testing" "testing/quick" + "time" "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/crypto" @@ -74,7 +75,7 @@ func TestBucket_bumpNoDuplicates(t *testing.T) { t.Parallel() cfg := &quick.Config{ MaxCount: 1000, - Rand: quickrand, + Rand: rand.New(rand.NewSource(time.Now().Unix())), Values: func(args []reflect.Value, rand *rand.Rand) { // generate a random list of nodes. this will be the content of the bucket. n := rand.Intn(bucketSize-1) + 1 @@ -205,7 +206,7 @@ func TestTable_closest(t *testing.T) { } return true } - if err := quick.Check(test, quickcfg); err != nil { + if err := quick.Check(test, quickcfg()); err != nil { t.Error(err) } } @@ -213,7 +214,7 @@ func TestTable_closest(t *testing.T) { func TestTable_ReadRandomNodesGetAll(t *testing.T) { cfg := &quick.Config{ MaxCount: 200, - Rand: quickrand, + Rand: rand.New(rand.NewSource(time.Now().Unix())), Values: func(args []reflect.Value, rand *rand.Rand) { args[0] = reflect.ValueOf(make([]*Node, rand.Intn(1000))) }, @@ -221,7 +222,7 @@ func TestTable_ReadRandomNodesGetAll(t *testing.T) { test := func(buf []*Node) bool { tab := newTable(nil, NodeID{}, &net.UDPAddr{}, "") for i := 0; i < len(buf); i++ { - ld := quickrand.Intn(len(tab.buckets)) + ld := cfg.Rand.Intn(len(tab.buckets)) tab.add([]*Node{nodeAtDistance(tab.self.sha, ld)}) } gotN := tab.ReadRandomNodes(buf) diff --git a/p2p/discover/udp_test.go b/p2p/discover/udp_test.go index 11fa31d7cc..b5d035a981 100644 --- a/p2p/discover/udp_test.go +++ b/p2p/discover/udp_test.go @@ -234,14 +234,12 @@ func TestUDP_findnodeMultiReply(t *testing.T) { func TestUDP_successfulPing(t *testing.T) { test := newUDPTest(t) + added := make(chan *Node, 1) + test.table.nodeAddedHook = func(n *Node) { added <- n } defer test.table.Close() - done := make(chan struct{}) - go func() { - // The remote side sends a ping packet to initiate the exchange. - test.packetIn(nil, pingPacket, &ping{From: testRemote, To: testLocalAnnounced, Version: Version, Expiration: futureExp}) - close(done) - }() + // The remote side sends a ping packet to initiate the exchange. + go test.packetIn(nil, pingPacket, &ping{From: testRemote, To: testLocalAnnounced, Version: Version, Expiration: futureExp}) // the ping is replied to. test.waitPacketOut(func(p *pong) { @@ -277,35 +275,26 @@ func TestUDP_successfulPing(t *testing.T) { }) test.packetIn(nil, pongPacket, &pong{Expiration: futureExp}) - // ping should return shortly after getting the pong packet. - <-done - - // check that the node was added. - rid := PubkeyID(&test.remotekey.PublicKey) - rnode := find(test.table, rid) - if rnode == nil { - t.Fatalf("node %v not found in table", rid) - } - if !bytes.Equal(rnode.IP, test.remoteaddr.IP) { - t.Errorf("node has wrong IP: got %v, want: %v", rnode.IP, test.remoteaddr.IP) - } - if int(rnode.UDP) != test.remoteaddr.Port { - t.Errorf("node has wrong UDP port: got %v, want: %v", rnode.UDP, test.remoteaddr.Port) - } - if rnode.TCP != testRemote.TCP { - t.Errorf("node has wrong TCP port: got %v, want: %v", rnode.TCP, testRemote.TCP) - } -} - -func find(tab *Table, id NodeID) *Node { - for _, b := range tab.buckets { - for _, e := range b.entries { - if e.ID == id { - return e - } + // the node should be added to the table shortly after getting the + // pong packet. + select { + case n := <-added: + rid := PubkeyID(&test.remotekey.PublicKey) + if n.ID != rid { + t.Errorf("node has wrong ID: got %v, want %v", n.ID, rid) } + if !bytes.Equal(n.IP, test.remoteaddr.IP) { + t.Errorf("node has wrong IP: got %v, want: %v", n.IP, test.remoteaddr.IP) + } + if int(n.UDP) != test.remoteaddr.Port { + t.Errorf("node has wrong UDP port: got %v, want: %v", n.UDP, test.remoteaddr.Port) + } + if n.TCP != testRemote.TCP { + t.Errorf("node has wrong TCP port: got %v, want: %v", n.TCP, testRemote.TCP) + } + case <-time.After(2 * time.Second): + t.Errorf("node was not added within 2 seconds") } - return nil } // dgramPipe is a fake UDP socket. It queues all sent datagrams.