From a89902e64981f50ce32b5762a799b777fc99e34e Mon Sep 17 00:00:00 2001 From: zelig Date: Wed, 27 Mar 2019 07:07:52 +0100 Subject: [PATCH] address PR review comments --- swarm/network/discovery.go | 2 +- swarm/network/discovery_test.go | 75 ++++++++++++++++----------------- 2 files changed, 37 insertions(+), 40 deletions(-) diff --git a/swarm/network/discovery.go b/swarm/network/discovery.go index e96a0323cd..2555c916cd 100644 --- a/swarm/network/discovery.go +++ b/swarm/network/discovery.go @@ -164,8 +164,8 @@ func (msg subPeersMsg) String() string { // otherwise this depth is just recorded on the peer, so that // subsequent new connections are sent iff they fall within the radius func (d *Peer) handleSubPeersMsg(msg *subPeersMsg) error { - // only do this once d.setDepth(msg.Depth) + // only send peers after the initial subPeersMsg if !d.sentPeers { var peers []*BzzAddr // iterate connection in ascending order of disctance from the remote address diff --git a/swarm/network/discovery_test.go b/swarm/network/discovery_test.go index 3ab2297f39..f1f3df0f96 100644 --- a/swarm/network/discovery_test.go +++ b/swarm/network/discovery_test.go @@ -37,7 +37,7 @@ import ( * - after connect, that outgoing subpeersmsg is sent * */ -func TestDiscovery(t *testing.T) { +func TestSubPeersMsg(t *testing.T) { params := NewHiveParams() s, pp, err := newHiveTester(t, params, 1, nil) if err != nil { @@ -70,24 +70,24 @@ func TestDiscovery(t *testing.T) { } const ( - maxPO = 10 - maxPeerPO = 8 + maxPO = 8 // PO of pivot and control; chosen to test enough cases but not run too long + maxPeerPO = 6 // pivot has no peers closer than this to the control peer ) // TestInitialPeersMsg tests if peersMsg response to incoming subPeersMsg is correct -func TestSubPeersMsg(t *testing.T) { +func TestInitialPeersMsg(t *testing.T) { for po := 0; po < maxPO; po++ { for depth := 0; depth < maxPO; depth++ { - t.Run(fmt.Sprintf("PO=%d,advetised depth=%d", po, depth), func(t *testing.T) { - testSubPeersMsg(t, po, depth) + t.Run(fmt.Sprintf("PO=%d,advertised depth=%d", po, depth), func(t *testing.T) { + testInitialPeersMsg(t, po, depth) }) } } } -// testSubPeersMsg tests that the correct set of peer info is sent +// testInitialPeersMsg tests that the correct set of peer info is sent // to another peer after receiving their subPeersMsg request -func testSubPeersMsg(t *testing.T, peerPO, peerDepth int) { +func testInitialPeersMsg(t *testing.T, peerPO, peerDepth int) { // generate random pivot address prvkey, err := crypto.GenerateKey() if err != nil { @@ -102,20 +102,17 @@ func testSubPeersMsg(t *testing.T, peerPO, peerDepth int) { // expected addrs in peersMsg response var expBzzAddrs []*BzzAddr - addrAt := func(a pot.Address, po int) []byte { - b := pot.RandomAddressAt(a, po) - return b[:] - } - connect := func(base pot.Address, po int) *BzzAddr { - on := addrAt(base, po) - peer := newDiscPeer(on) + connect := func(a pot.Address, po int) *BzzAddr { + peer := newDiscPeer(pot.RandomAddressAt(a, po)) hive.On(peer) return peer.BzzAddr } - register := func(base pot.Address, po int) { - hive.Register(&BzzAddr{OAddr: addrAt(base, po)}) + register := func(a pot.Address, po int) { + addr := pot.RandomAddressAt(a, po) + hive.Register(&BzzAddr{OAddr: addr[:]}) } + // generate connected and just registered peers for po := maxPeerPO; po >= 0; po-- { // create a fake connected peer at po from peerAddr on := connect(peerAddr, po) @@ -136,19 +133,17 @@ func testSubPeersMsg(t *testing.T, peerPO, peerDepth int) { // peerID to use in the protocol tester testExchange expect/trigger peerID := s.Nodes[0].ID() - - // now we need to wait until the tester's control peer appears in the hive - // so the protocol started - ticker := time.NewTicker(10 * time.Millisecond) - attempts := 100 - for range ticker.C { - if _, found := hive.peers[peerID]; found { + // block until control peer is found among hive peers + found := false + for attempts := 0; attempts < 20; attempts++ { + if _, found = hive.peers[peerID]; found { break } - attempts-- - if attempts == 0 { - t.Fatal("timeout waiting for control peer to be in kademlia") - } + time.Sleep(1 * time.Millisecond) + } + + if !found { + t.Fatal("timeout waiting for peer connection to start") } // pivotDepth is the advertised depth of the pivot node we expect in the outgoing subPeersMsg @@ -189,30 +184,32 @@ func testSubPeersMsg(t *testing.T, peerPO, peerDepth int) { // for values MaxPeerPO < peerPO < MaxPO the pivot has no peers to offer to the control peer // in this case, no peersMsg will be sent out, and we would run into a time out + if len(expBzzAddrs) == 0 { + if err != nil { + if err.Error() != "exchange #1 \"trigger subPeersMsg and expect peersMsg\": timed out" { + t.Fatalf("expected timeout, got %v", err) + } + return + } + t.Fatalf("expected timeout, got no error") + } + if err != nil { - if len(expBzzAddrs) > 0 { - t.Fatal(err) - } else if err.Error() != "exchange #1 \"trigger subPeersMsg and expect peersMsg\": timed out" { - t.Fatalf("expected timeout, got %v", err) - } - } else { - if len(expBzzAddrs) == 0 { - t.Fatalf("expected timeout, got no error") - } + t.Fatal(err) } } // as we are not creating a real node via the protocol, // we need to create the discovery peer objects for the additional kademlia // nodes manually -func newDiscPeer(addr []byte) *Peer { +func newDiscPeer(addr pot.Address) *Peer { pKey, err := ecdsa.GenerateKey(crypto.S256(), rand.Reader) if err != nil { panic(err.Error()) } pubKey := pKey.PublicKey nod := enode.NewV4(&pubKey, net.IPv4(127, 0, 0, 1), 0, 0) - bzzAddr := &BzzAddr{OAddr: addr, UAddr: []byte(nod.String())} + bzzAddr := &BzzAddr{OAddr: addr[:], UAddr: []byte(nod.String())} id := nod.ID() p2pPeer := p2p.NewPeer(id, id.String(), nil) return NewPeer(&BzzPeer{