p2p/simulations: ensure method conformity for Network

Connect* methods were moved to p2p/simulations.Network from
swarm/network/simulation. However these new methods did not follow
the pattern of Network methods, i.e., all exported method locks
the whole Network either for read or write.

p2p/simulations/connect.go

@@ -32,6 +32,9 @@ var (
 // It is useful when constructing a chain network topology
 // when Network adds and removes nodes dynamically.
 func (net *Network) ConnectToLastNode(id enode.ID) (err error) {
+	net.lock.Lock()
+	defer net.lock.Unlock()
+
 	ids := net.getUpNodeIDs()
 	l := len(ids)
 	if l < 2 {
@@ -41,29 +44,35 @@ func (net *Network) ConnectToLastNode(id enode.ID) (err error) {
 	if last == id {
 		last = ids[l-2]
 	}
-	return net.connect(last, id)
+	return net.connectNotConnected(last, id)
 }
 
 // ConnectToRandomNode connects the node with provided NodeID
 // to a random node that is up.
 func (net *Network) ConnectToRandomNode(id enode.ID) (err error) {
-	selected := net.GetRandomUpNode(id)
+	net.lock.Lock()
+	defer net.lock.Unlock()
+
+	selected := net.getRandomUpNode(id)
 	if selected == nil {
 		return ErrNodeNotFound
 	}
-	return net.connect(selected.ID(), id)
+	return net.connectNotConnected(selected.ID(), id)
 }
 
 // ConnectNodesFull connects all nodes one to another.
 // It provides a complete connectivity in the network
 // which should be rarely needed.
 func (net *Network) ConnectNodesFull(ids []enode.ID) (err error) {
+	net.lock.Lock()
+	defer net.lock.Unlock()
+
 	if ids == nil {
 		ids = net.getUpNodeIDs()
 	}
 	for i, lid := range ids {
 		for _, rid := range ids[i+1:] {
-			if err = net.connect(lid, rid); err != nil {
+			if err = net.connectNotConnected(lid, rid); err != nil {
 				return err
 			}
 		}
@@ -74,12 +83,19 @@ func (net *Network) ConnectNodesFull(ids []enode.ID) (err error) {
 // ConnectNodesChain connects all nodes in a chain topology.
 // If ids argument is nil, all nodes that are up will be connected.
 func (net *Network) ConnectNodesChain(ids []enode.ID) (err error) {
+	net.lock.Lock()
+	defer net.lock.Unlock()
+
+	return net.connectNodesChain(ids)
+}
+
+func (net *Network) connectNodesChain(ids []enode.ID) (err error) {
 	if ids == nil {
 		ids = net.getUpNodeIDs()
 	}
 	l := len(ids)
 	for i := 0; i < l-1; i++ {
-		if err := net.connect(ids[i], ids[i+1]); err != nil {
+		if err := net.connectNotConnected(ids[i], ids[i+1]); err != nil {
 			return err
 		}
 	}
@@ -89,6 +105,9 @@ func (net *Network) ConnectNodesChain(ids []enode.ID) (err error) {
 // ConnectNodesRing connects all nodes in a ring topology.
 // If ids argument is nil, all nodes that are up will be connected.
 func (net *Network) ConnectNodesRing(ids []enode.ID) (err error) {
+	net.lock.Lock()
+	defer net.lock.Unlock()
+
 	if ids == nil {
 		ids = net.getUpNodeIDs()
 	}
@@ -96,15 +115,18 @@ func (net *Network) ConnectNodesRing(ids []enode.ID) (err error) {
 	if l < 2 {
 		return nil
 	}
-	if err := net.ConnectNodesChain(ids); err != nil {
+	if err := net.connectNodesChain(ids); err != nil {
 		return err
 	}
-	return net.connect(ids[l-1], ids[0])
+	return net.connectNotConnected(ids[l-1], ids[0])
 }
 
 // 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(ids []enode.ID, center enode.ID) (err error) {
+	net.lock.Lock()
+	defer net.lock.Unlock()
+
 	if ids == nil {
 		ids = net.getUpNodeIDs()
 	}
@@ -112,16 +134,16 @@ func (net *Network) ConnectNodesStar(ids []enode.ID, center enode.ID) (err error
 		if center == id {
 			continue
 		}
-		if err := net.connect(center, id); err != nil {
+		if err := net.connectNotConnected(center, id); err != nil {
 			return err
 		}
 	}
 	return nil
 }
 
-// connect connects two nodes but ignores already connected error.
+// connectByIgnoringAlreadyConnected connects two nodes but ignores already connected error.
-func (net *Network) connect(oneID, otherID enode.ID) error {
+func (net *Network) connectNotConnected(oneID, otherID enode.ID) error {
-	return ignoreAlreadyConnectedErr(net.Connect(oneID, otherID))
+	return ignoreAlreadyConnectedErr(net.connect(oneID, otherID))
 }
 
 func ignoreAlreadyConnectedErr(err error) error {

p2p/simulations/network.go

@@ -278,8 +278,14 @@ func (net *Network) Stop(id enode.ID) error {
 // Connect connects two nodes together by calling the "admin_addPeer" RPC
 // method on the "one" node so that it connects to the "other" node
 func (net *Network) Connect(oneID, otherID enode.ID) error {
+	net.lock.Lock()
+	defer net.lock.Unlock()
+	return net.connect(oneID, otherID)
+}
+
+func (net *Network) connect(oneID, otherID enode.ID) error {
 	log.Debug("Connecting nodes with addPeer", "id", oneID, "other", otherID)
-	conn, err := net.InitConn(oneID, otherID)
+	conn, err := net.initConn(oneID, otherID)
 	if err != nil {
 		return err
 	}
@@ -419,6 +425,11 @@ func (net *Network) getNodes() (nodes []*Node) {
 func (net *Network) GetRandomUpNode(excludeIDs ...enode.ID) *Node {
 	net.lock.RLock()
 	defer net.lock.RUnlock()
+	return net.getRandomUpNode(excludeIDs...)
+}
+
+// GetRandomUpNode returns a random node on the network, which is running.
+func (net *Network) getRandomUpNode(excludeIDs ...enode.ID) *Node {
 	return net.getRandomNode(net.getUpNodeIDs(), excludeIDs)
 }
 
@@ -532,6 +543,10 @@ func (net *Network) getConn(oneID, otherID enode.ID) *Conn {
 func (net *Network) InitConn(oneID, otherID enode.ID) (*Conn, error) {
 	net.lock.Lock()
 	defer net.lock.Unlock()
+	return net.initConn(oneID, otherID)
+}
+
+func (net *Network) initConn(oneID, otherID enode.ID) (*Conn, error) {
 	if oneID == otherID {
 		return nil, fmt.Errorf("refusing to connect to self %v", oneID)
 	}