From 2eb838ed9776c9c3ec922e1116a5d50babda31c5 Mon Sep 17 00:00:00 2001 From: Ferenc Szabo Date: Fri, 11 Jan 2019 10:23:45 +0100 Subject: [PATCH] p2p/simulations: eliminate concept of pivot (#18426) --- p2p/simulations/adapters/inproc.go | 14 ------ p2p/simulations/connect.go | 52 ++------------------- p2p/simulations/connect_test.go | 65 +++++++++----------------- p2p/simulations/network.go | 2 - swarm/network/simulation/node.go | 21 +-------- swarm/network/simulation/node_test.go | 39 ---------------- swarm/network/simulation/simulation.go | 1 - 7 files changed, 28 insertions(+), 166 deletions(-) diff --git a/p2p/simulations/adapters/inproc.go b/p2p/simulations/adapters/inproc.go index d122cd6487..eada9579ed 100644 --- a/p2p/simulations/adapters/inproc.go +++ b/p2p/simulations/adapters/inproc.go @@ -351,17 +351,3 @@ func (sn *SimNode) NodeInfo() *p2p.NodeInfo { } return server.NodeInfo() } - -func setSocketBuffer(conn net.Conn, socketReadBuffer int, socketWriteBuffer int) error { - if v, ok := conn.(*net.UnixConn); ok { - err := v.SetReadBuffer(socketReadBuffer) - if err != nil { - return err - } - err = v.SetWriteBuffer(socketWriteBuffer) - if err != nil { - return err - } - } - return nil -} diff --git a/p2p/simulations/connect.go b/p2p/simulations/connect.go index 606dda0567..bb7e7999a1 100644 --- a/p2p/simulations/connect.go +++ b/p2p/simulations/connect.go @@ -25,21 +25,8 @@ import ( var ( ErrNodeNotFound = errors.New("node not found") - ErrNoPivotNode = errors.New("no pivot node set") ) -// ConnectToPivotNode connects the node with provided NodeID -// to the pivot node, already set by Network.SetPivotNode method. -// It is useful when constructing a star network topology -// when Network adds and removes nodes dynamically. -func (net *Network) ConnectToPivotNode(id enode.ID) (err error) { - pivot := net.GetPivotNode() - if pivot == nil { - return ErrNoPivotNode - } - return net.connect(pivot.ID(), id) -} - // ConnectToLastNode connects the node with provided NodeID // to the last node that is up, and avoiding connection to self. // It is useful when constructing a chain network topology @@ -115,35 +102,23 @@ func (net *Network) ConnectNodesRing(ids []enode.ID) (err error) { return net.connect(ids[l-1], ids[0]) } -// ConnectNodesStar connects all nodes in a star topology -// with the center at provided NodeID. +// ConnectNodesStar connects all nodes into a star topology // If ids argument is nil, all nodes that are up will be connected. -func (net *Network) ConnectNodesStar(pivot enode.ID, ids []enode.ID) (err error) { +func (net *Network) ConnectNodesStar(ids []enode.ID, center enode.ID) (err error) { if ids == nil { ids = net.getUpNodeIDs() } for _, id := range ids { - if pivot == id { + if center == id { continue } - if err := net.connect(pivot, id); err != nil { + if err := net.connect(center, id); err != nil { return err } } return nil } -// ConnectNodesStarPivot connects all nodes in a star topology -// with the center at already set pivot node. -// If ids argument is nil, all nodes that are up will be connected. -func (net *Network) ConnectNodesStarPivot(ids []enode.ID) (err error) { - pivot := net.GetPivotNode() - if pivot == nil { - return ErrNoPivotNode - } - return net.ConnectNodesStar(pivot.ID(), ids) -} - // connect connects two nodes but ignores already connected error. func (net *Network) connect(oneID, otherID enode.ID) error { return ignoreAlreadyConnectedErr(net.Connect(oneID, otherID)) @@ -155,22 +130,3 @@ func ignoreAlreadyConnectedErr(err error) error { } return err } - -// SetPivotNode sets the NodeID of the network's pivot node. -// Pivot node is just a specific node that should be treated -// differently then other nodes in test. SetPivotNode and -// GetPivotNode are just a convenient functions to set and -// retrieve it. -func (net *Network) SetPivotNode(id enode.ID) { - net.lock.Lock() - defer net.lock.Unlock() - net.pivotNodeID = id -} - -// GetPivotNode returns NodeID of the pivot node set by -// Network.SetPivotNode method. -func (net *Network) GetPivotNode() (node *Node) { - net.lock.RLock() - defer net.lock.RUnlock() - return net.getNode(net.pivotNodeID) -} diff --git a/p2p/simulations/connect_test.go b/p2p/simulations/connect_test.go index fd2bf5c845..32d18347d8 100644 --- a/p2p/simulations/connect_test.go +++ b/p2p/simulations/connect_test.go @@ -58,24 +58,6 @@ func newTestNetwork(t *testing.T, nodeCount int) (*Network, []enode.ID) { return network, ids } -func TestConnectToPivotNode(t *testing.T) { - net, ids := newTestNetwork(t, 2) - defer net.Shutdown() - - pivot := ids[0] - net.SetPivotNode(pivot) - - other := ids[1] - err := net.ConnectToPivotNode(other) - if err != nil { - t.Fatal(err) - } - - if net.GetConn(pivot, other) == nil { - t.Error("pivot and the other node are not connected") - } -} - func TestConnectToLastNode(t *testing.T) { net, ids := newTestNetwork(t, 10) defer net.Shutdown() @@ -125,15 +107,30 @@ func TestConnectToRandomNode(t *testing.T) { } func TestConnectNodesFull(t *testing.T) { - net, ids := newTestNetwork(t, 12) - defer net.Shutdown() - - err := net.ConnectNodesFull(ids) - if err != nil { - t.Fatal(err) + tests := []struct { + name string + nodeCount int + }{ + {name: "no node", nodeCount: 0}, + {name: "single node", nodeCount: 1}, + {name: "2 nodes", nodeCount: 2}, + {name: "3 nodes", nodeCount: 3}, + {name: "even number of nodes", nodeCount: 12}, + {name: "odd number of nodes", nodeCount: 13}, } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + net, ids := newTestNetwork(t, test.nodeCount) + defer net.Shutdown() + + err := net.ConnectNodesFull(ids) + if err != nil { + t.Fatal(err) + } - VerifyFull(t, net, ids) + VerifyFull(t, net, ids) + }) + } } func TestConnectNodesChain(t *testing.T) { @@ -166,23 +163,7 @@ func TestConnectNodesStar(t *testing.T) { pivotIndex := 2 - err := net.ConnectNodesStar(ids[pivotIndex], ids) - if err != nil { - t.Fatal(err) - } - - VerifyStar(t, net, ids, pivotIndex) -} - -func TestConnectNodesStarPivot(t *testing.T) { - net, ids := newTestNetwork(t, 10) - defer net.Shutdown() - - pivotIndex := 4 - - net.SetPivotNode(ids[pivotIndex]) - - err := net.ConnectNodesStarPivot(ids) + err := net.ConnectNodesStar(ids, ids[pivotIndex]) if err != nil { t.Fatal(err) } diff --git a/p2p/simulations/network.go b/p2p/simulations/network.go index a711fd78e3..86f7dc9bef 100644 --- a/p2p/simulations/network.go +++ b/p2p/simulations/network.go @@ -58,8 +58,6 @@ type Network struct { Conns []*Conn `json:"conns"` connMap map[string]int - pivotNodeID enode.ID - nodeAdapter adapters.NodeAdapter events event.Feed lock sync.RWMutex diff --git a/swarm/network/simulation/node.go b/swarm/network/simulation/node.go index e8d4d6d948..08eb835247 100644 --- a/swarm/network/simulation/node.go +++ b/swarm/network/simulation/node.go @@ -188,7 +188,7 @@ func (s *Simulation) AddNodesAndConnectStar(count int, opts ...AddNodeOption) (i if err != nil { return nil, err } - err = s.Net.ConnectNodesStar(ids[0], ids[1:]) + err = s.Net.ConnectNodesStar(ids[1:], ids[0]) if err != nil { return nil, err } @@ -241,25 +241,6 @@ func (s *Simulation) UploadSnapshot(snapshotFile string, opts ...AddNodeOption) return nil } -// SetPivotNode sets the NodeID of the network's pivot node. -// Pivot node is just a specific node that should be treated -// differently then other nodes in test. SetPivotNode and -// PivotNodeID are just a convenient functions to set and -// retrieve it. -func (s *Simulation) SetPivotNode(id enode.ID) { - s.mu.Lock() - defer s.mu.Unlock() - s.pivotNodeID = &id -} - -// PivotNodeID returns NodeID of the pivot node set by -// Simulation.SetPivotNode method. -func (s *Simulation) PivotNodeID() (id *enode.ID) { - s.mu.Lock() - defer s.mu.Unlock() - return s.pivotNodeID -} - // StartNode starts a node by NodeID. func (s *Simulation) StartNode(id enode.ID) (err error) { return s.Net.Start(id) diff --git a/swarm/network/simulation/node_test.go b/swarm/network/simulation/node_test.go index 8da32cf37f..dc9189c911 100644 --- a/swarm/network/simulation/node_test.go +++ b/swarm/network/simulation/node_test.go @@ -314,45 +314,6 @@ func TestUploadSnapshot(t *testing.T) { log.Debug("Done.") } -func TestPivotNode(t *testing.T) { - sim := New(noopServiceFuncMap) - defer sim.Close() - - id, err := sim.AddNode() - if err != nil { - t.Fatal(err) - } - - id2, err := sim.AddNode() - if err != nil { - t.Fatal(err) - } - - if sim.PivotNodeID() != nil { - t.Error("expected no pivot node") - } - - sim.SetPivotNode(id) - - pid := sim.PivotNodeID() - - if pid == nil { - t.Error("pivot node not set") - } else if *pid != id { - t.Errorf("expected pivot node %s, got %s", id, *pid) - } - - sim.SetPivotNode(id2) - - pid = sim.PivotNodeID() - - if pid == nil { - t.Error("pivot node not set") - } else if *pid != id2 { - t.Errorf("expected pivot node %s, got %s", id2, *pid) - } -} - func TestStartStopNode(t *testing.T) { sim := New(noopServiceFuncMap) defer sim.Close() diff --git a/swarm/network/simulation/simulation.go b/swarm/network/simulation/simulation.go index 13c5b1c575..e18d19a67c 100644 --- a/swarm/network/simulation/simulation.go +++ b/swarm/network/simulation/simulation.go @@ -46,7 +46,6 @@ type Simulation struct { serviceNames []string cleanupFuncs []func() buckets map[enode.ID]*sync.Map - pivotNodeID *enode.ID shutdownWG sync.WaitGroup done chan struct{} mu sync.RWMutex