From a47e1685c03ecb715f17a90aba57accdec3d8d27 Mon Sep 17 00:00:00 2001 From: zelig Date: Mon, 1 Apr 2019 07:46:09 +0200 Subject: [PATCH] swarm/network: address PR comments on TestInitialPeersMsg * eliminate *testing.T argument from bzz/hive protocoltesters * add sorting (only runs in test code) on peersMsg payload * add random (0 to MaxPeersPerPO) peers for each po * add extra peers closer to pivot than control --- swarm/network/discovery.go | 8 +++- swarm/network/discovery_test.go | 66 +++++++++++++++++++++++++-------- swarm/network/hive_test.go | 6 +-- swarm/network/protocol_test.go | 6 +-- 4 files changed, 63 insertions(+), 23 deletions(-) diff --git a/swarm/network/discovery.go b/swarm/network/discovery.go index 2555c916cd..54ecf257c3 100644 --- a/swarm/network/discovery.go +++ b/swarm/network/discovery.go @@ -26,6 +26,8 @@ import ( // discovery bzz extension for requesting and relaying node address records +var sortPeers = noSortPeers + // Peer wraps BzzPeer and embeds Kademlia overlay connectivity driver type Peer struct { *BzzPeer @@ -181,7 +183,7 @@ func (d *Peer) handleSubPeersMsg(msg *subPeersMsg) error { }) // if useful peers are found, send them over if len(peers) > 0 { - go d.Send(context.TODO(), &peersMsg{Peers: peers}) + go d.Send(context.TODO(), &peersMsg{Peers: sortPeers(peers)}) } } d.sentPeers = true @@ -212,3 +214,7 @@ func (d *Peer) setDepth(depth uint8) { defer d.mtx.Unlock() d.depth = depth } + +func noSortPeers(peers []*BzzAddr) []*BzzAddr { + return peers +} diff --git a/swarm/network/discovery_test.go b/swarm/network/discovery_test.go index f1f3df0f96..04e1b36fed 100644 --- a/swarm/network/discovery_test.go +++ b/swarm/network/discovery_test.go @@ -18,9 +18,12 @@ package network import ( "crypto/ecdsa" - "crypto/rand" + crand "crypto/rand" + "encoding/binary" "fmt" + "math/rand" "net" + "sort" "testing" "time" @@ -39,7 +42,7 @@ import ( */ func TestSubPeersMsg(t *testing.T) { params := NewHiveParams() - s, pp, err := newHiveTester(t, params, 1, nil) + s, pp, err := newHiveTester(params, 1, nil) if err != nil { t.Fatal(err) } @@ -70,8 +73,9 @@ func TestSubPeersMsg(t *testing.T) { } const ( - 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 + 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 + maxPeersPerPO = 3 ) // TestInitialPeersMsg tests if peersMsg response to incoming subPeersMsg is correct @@ -93,6 +97,11 @@ func testInitialPeersMsg(t *testing.T, peerPO, peerDepth int) { if err != nil { t.Fatal(err) } + + defer func(orig func([]*BzzAddr) []*BzzAddr) { + sortPeers = orig + }(sortPeers) + sortPeers = testSortPeers pivotAddr := pot.NewAddressFromBytes(PrivateKeyToBzzKey(prvkey)) // generate control peers address at peerPO wrt pivot peerAddr := pot.RandomAddressAt(pivotAddr, peerPO) @@ -102,10 +111,17 @@ func testInitialPeersMsg(t *testing.T, peerPO, peerDepth int) { // expected addrs in peersMsg response var expBzzAddrs []*BzzAddr - connect := func(a pot.Address, po int) *BzzAddr { - peer := newDiscPeer(pot.RandomAddressAt(a, po)) - hive.On(peer) - return peer.BzzAddr + connect := func(a pot.Address, po int) (addrs []*BzzAddr) { + n := rand.Intn(maxPeersPerPO) + for i := 0; i < n; i++ { + peer, err := newDiscPeer(pot.RandomAddressAt(a, po)) + if err != nil { + t.Fatal(err) + } + hive.On(peer) + addrs = append(addrs, peer.BzzAddr) + } + return addrs } register := func(a pot.Address, po int) { addr := pot.RandomAddressAt(a, po) @@ -115,18 +131,26 @@ func testInitialPeersMsg(t *testing.T, peerPO, peerDepth int) { // 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) + ons := connect(peerAddr, po) // create a fake registered address at po from peerAddr register(peerAddr, po) // we collect expected peer addresses only up till peerPO if po < peerDepth { continue } - expBzzAddrs = append(expBzzAddrs, on) + expBzzAddrs = append(expBzzAddrs, ons...) + } + + // add extra connections closer to pivot than control + for po := peerPO + 1; po < maxPO; po++ { + ons := connect(pivotAddr, po) + if peerDepth <= peerPO { + expBzzAddrs = append(expBzzAddrs, ons...) + } } // create a special bzzBaseTester in which we can associate `enode.ID` to the `bzzAddr` we created above - s, _, err := newBzzBaseTesterWithAddrs(t, prvkey, [][]byte{peerAddr[:]}, DiscoverySpec, hive.Run) + s, _, err := newBzzBaseTesterWithAddrs(prvkey, [][]byte{peerAddr[:]}, DiscoverySpec, hive.Run) if err != nil { t.Fatal(err) } @@ -175,7 +199,7 @@ func testInitialPeersMsg(t *testing.T, peerPO, peerDepth int) { Expects: []p2ptest.Expect{ { Code: 0, - Msg: &peersMsg{Peers: expBzzAddrs}, + Msg: &peersMsg{Peers: testSortPeers(expBzzAddrs)}, Peer: peerID, Timeout: 100 * time.Millisecond, }, @@ -199,13 +223,23 @@ func testInitialPeersMsg(t *testing.T, peerPO, peerDepth int) { } } +func testSortPeers(peers []*BzzAddr) []*BzzAddr { + comp := func(i, j int) bool { + vi := binary.BigEndian.Uint64(peers[i].OAddr) + vj := binary.BigEndian.Uint64(peers[j].OAddr) + return vi < vj + } + sort.Slice(peers, comp) + return peers +} + // 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 pot.Address) *Peer { - pKey, err := ecdsa.GenerateKey(crypto.S256(), rand.Reader) +func newDiscPeer(addr pot.Address) (*Peer, error) { + pKey, err := ecdsa.GenerateKey(crypto.S256(), crand.Reader) if err != nil { - panic(err.Error()) + return nil, err } pubKey := pKey.PublicKey nod := enode.NewV4(&pubKey, net.IPv4(127, 0, 0, 1), 0, 0) @@ -215,7 +249,7 @@ func newDiscPeer(addr pot.Address) *Peer { return NewPeer(&BzzPeer{ Peer: protocols.NewPeer(p2pPeer, &dummyMsgRW{}, DiscoverySpec), BzzAddr: bzzAddr, - }, nil) + }, nil), nil } type dummyMsgRW struct{} diff --git a/swarm/network/hive_test.go b/swarm/network/hive_test.go index 3ae86185f4..d03db42bce 100644 --- a/swarm/network/hive_test.go +++ b/swarm/network/hive_test.go @@ -28,7 +28,7 @@ import ( "github.com/ethereum/go-ethereum/swarm/state" ) -func newHiveTester(t *testing.T, params *HiveParams, n int, store state.Store) (*bzzTester, *Hive, error) { +func newHiveTester(params *HiveParams, n int, store state.Store) (*bzzTester, *Hive, error) { // setup prvkey, err := crypto.GenerateKey() if err != nil { @@ -38,7 +38,7 @@ func newHiveTester(t *testing.T, params *HiveParams, n int, store state.Store) ( to := NewKademlia(addr, NewKadParams()) pp := NewHive(params, to, store) // hive - bt, err := newBzzBaseTester(t, n, prvkey, DiscoverySpec, pp.Run) + bt, err := newBzzBaseTester(n, prvkey, DiscoverySpec, pp.Run) if err != nil { return nil, nil, err } @@ -49,7 +49,7 @@ func newHiveTester(t *testing.T, params *HiveParams, n int, store state.Store) ( // and that the peer connection exists afterwards func TestRegisterAndConnect(t *testing.T) { params := NewHiveParams() - s, pp, err := newHiveTester(t, params, 1, nil) + s, pp, err := newHiveTester(params, 1, nil) if err != nil { t.Fatal(err) } diff --git a/swarm/network/protocol_test.go b/swarm/network/protocol_test.go index 75f9524505..b562a42534 100644 --- a/swarm/network/protocol_test.go +++ b/swarm/network/protocol_test.go @@ -73,17 +73,17 @@ func HandshakeMsgExchange(lhs, rhs *HandshakeMsg, id enode.ID) []p2ptest.Exchang } } -func newBzzBaseTester(t *testing.T, n int, prvkey *ecdsa.PrivateKey, spec *protocols.Spec, run func(*BzzPeer) error) (*bzzTester, error) { +func newBzzBaseTester(n int, prvkey *ecdsa.PrivateKey, spec *protocols.Spec, run func(*BzzPeer) error) (*bzzTester, error) { var addrs [][]byte for i := 0; i < n; i++ { addr := pot.RandomAddress() addrs = append(addrs, addr[:]) } - pt, _, err := newBzzBaseTesterWithAddrs(t, prvkey, addrs, spec, run) + pt, _, err := newBzzBaseTesterWithAddrs(prvkey, addrs, spec, run) return pt, err } -func newBzzBaseTesterWithAddrs(t *testing.T, prvkey *ecdsa.PrivateKey, addrs [][]byte, spec *protocols.Spec, run func(*BzzPeer) error) (*bzzTester, [][]byte, error) { +func newBzzBaseTesterWithAddrs(prvkey *ecdsa.PrivateKey, addrs [][]byte, spec *protocols.Spec, run func(*BzzPeer) error) (*bzzTester, [][]byte, error) { n := len(addrs) cs := make(map[enode.ID]chan bool)