p2p/simulations: eliminate concept of pivot (#18426)

Author: frncmx

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
-}

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)
-}

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)
 	}

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

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)

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()

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