From 44a9741e48ffb20e4d6aba24740dad3b069e8c58 Mon Sep 17 00:00:00 2001 From: Ferenc Szabo Date: Wed, 19 Dec 2018 13:42:46 +0100 Subject: [PATCH 01/18] swarm/network: remove unused opts param of AddNodes* function Only AddNode() had some valid use cases for AddNodeOption, but yet the params were propagated everywhere. --- swarm/network/simulation/node.go | 27 ++++++++++++-------------- swarm/network/simulation/simulation.go | 2 +- 2 files changed, 13 insertions(+), 16 deletions(-) diff --git a/swarm/network/simulation/node.go b/swarm/network/simulation/node.go index 24b6599762..a48dd96ddf 100644 --- a/swarm/network/simulation/node.go +++ b/swarm/network/simulation/node.go @@ -105,10 +105,10 @@ func (s *Simulation) AddNode(opts ...AddNodeOption) (id enode.ID, err error) { // AddNodes creates new nodes with random configurations, // applies provided options to the config and adds nodes to network. -func (s *Simulation) AddNodes(count int, opts ...AddNodeOption) (ids []enode.ID, err error) { +func (s *Simulation) AddNodes(count int) (ids []enode.ID, err error) { ids = make([]enode.ID, 0, count) for i := 0; i < count; i++ { - id, err := s.AddNode(opts...) + id, err := s.AddNode() if err != nil { return nil, err } @@ -119,11 +119,11 @@ func (s *Simulation) AddNodes(count int, opts ...AddNodeOption) (ids []enode.ID, // AddNodesAndConnectFull is a helpper method that combines // AddNodes and ConnectNodesFull. Only new nodes will be connected. -func (s *Simulation) AddNodesAndConnectFull(count int, opts ...AddNodeOption) (ids []enode.ID, err error) { +func (s *Simulation) AddNodesAndConnectFull(count int) (ids []enode.ID, err error) { if count < 2 { return nil, errors.New("count of nodes must be at least 2") } - ids, err = s.AddNodes(count, opts...) + ids, err = s.AddNodes(count) if err != nil { return nil, err } @@ -137,11 +137,11 @@ func (s *Simulation) AddNodesAndConnectFull(count int, opts ...AddNodeOption) (i // AddNodesAndConnectChain is a helpper method that combines // AddNodes and ConnectNodesChain. The chain will be continued from the last // added node, if there is one in simulation using ConnectToLastNode method. -func (s *Simulation) AddNodesAndConnectChain(count int, opts ...AddNodeOption) (ids []enode.ID, err error) { +func (s *Simulation) AddNodesAndConnectChain(count int) (ids []enode.ID, err error) { if count < 2 { return nil, errors.New("count of nodes must be at least 2") } - id, err := s.AddNode(opts...) + id, err := s.AddNode() if err != nil { return nil, err } @@ -149,7 +149,7 @@ func (s *Simulation) AddNodesAndConnectChain(count int, opts ...AddNodeOption) ( if err != nil { return nil, err } - ids, err = s.AddNodes(count-1, opts...) + ids, err = s.AddNodes(count - 1) if err != nil { return nil, err } @@ -163,11 +163,11 @@ func (s *Simulation) AddNodesAndConnectChain(count int, opts ...AddNodeOption) ( // AddNodesAndConnectRing is a helpper method that combines // AddNodes and ConnectNodesRing. -func (s *Simulation) AddNodesAndConnectRing(count int, opts ...AddNodeOption) (ids []enode.ID, err error) { +func (s *Simulation) AddNodesAndConnectRing(count int) (ids []enode.ID, err error) { if count < 2 { return nil, errors.New("count of nodes must be at least 2") } - ids, err = s.AddNodes(count, opts...) + ids, err = s.AddNodes(count) if err != nil { return nil, err } @@ -180,11 +180,11 @@ func (s *Simulation) AddNodesAndConnectRing(count int, opts ...AddNodeOption) (i // AddNodesAndConnectStar is a helpper method that combines // AddNodes and ConnectNodesStar. -func (s *Simulation) AddNodesAndConnectStar(count int, opts ...AddNodeOption) (ids []enode.ID, err error) { +func (s *Simulation) AddNodesAndConnectStar(count int) (ids []enode.ID, err error) { if count < 2 { return nil, errors.New("count of nodes must be at least 2") } - ids, err = s.AddNodes(count, opts...) + ids, err = s.AddNodes(count) if err != nil { return nil, err } @@ -198,7 +198,7 @@ func (s *Simulation) AddNodesAndConnectStar(count int, opts ...AddNodeOption) (i //UploadSnapshot uploads a snapshot to the simulation //This method tries to open the json file provided, applies the config to all nodes //and then loads the snapshot into the Simulation network -func (s *Simulation) UploadSnapshot(snapshotFile string, opts ...AddNodeOption) error { +func (s *Simulation) UploadSnapshot(snapshotFile string) error { f, err := os.Open(snapshotFile) if err != nil { return err @@ -225,9 +225,6 @@ func (s *Simulation) UploadSnapshot(snapshotFile string, opts ...AddNodeOption) for _, n := range snap.Nodes { n.Node.Config.EnableMsgEvents = true n.Node.Config.Services = s.serviceNames - for _, o := range opts { - o(n.Node.Config) - } } log.Info("Waiting for p2p connections to be established...") diff --git a/swarm/network/simulation/simulation.go b/swarm/network/simulation/simulation.go index 106eeb71e9..5f39b59117 100644 --- a/swarm/network/simulation/simulation.go +++ b/swarm/network/simulation/simulation.go @@ -38,7 +38,7 @@ var ( // Simulation provides methods on network, nodes and services // to manage them. type Simulation struct { - // Net is exposed as a way to access lower level functionalities + // Net is exposed as a way to access lower level functionality // of p2p/simulations.Network. Net *simulations.Network From b407261ebfcd0af3871eda95a8e4837b324fa65b Mon Sep 17 00:00:00 2001 From: Ferenc Szabo Date: Wed, 19 Dec 2018 14:05:32 +0100 Subject: [PATCH 02/18] p2p/simulations: add more test cases for ConnectNodesFull() As I surprised myself it works even with 0 or 1 nodes. --- p2p/simulations/connect_test.go | 29 ++++++++++++++++++++++------- 1 file changed, 22 insertions(+), 7 deletions(-) diff --git a/p2p/simulations/connect_test.go b/p2p/simulations/connect_test.go index 36f9442a8a..7969c2d5d7 100644 --- a/p2p/simulations/connect_test.go +++ b/p2p/simulations/connect_test.go @@ -124,15 +124,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: 2}, + {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) { From eec450b5da317af41b0f33a510ac678bfdf97051 Mon Sep 17 00:00:00 2001 From: Ferenc Szabo Date: Wed, 19 Dec 2018 14:21:01 +0100 Subject: [PATCH 03/18] swarm/network: remove AddNodesAndConnectFull() method Added no useful functionality as it was just combining 2 simple methods and the function was only used in its own test. --- p2p/simulations/connect_test.go | 2 +- p2p/simulations/test.go | 36 +++++++++++++-------------- swarm/network/simulation/node.go | 17 ------------- swarm/network/simulation/node_test.go | 14 ----------- 4 files changed, 19 insertions(+), 50 deletions(-) diff --git a/p2p/simulations/connect_test.go b/p2p/simulations/connect_test.go index 7969c2d5d7..0029b44115 100644 --- a/p2p/simulations/connect_test.go +++ b/p2p/simulations/connect_test.go @@ -145,7 +145,7 @@ func TestConnectNodesFull(t *testing.T) { t.Fatal(err) } - VerifyFull(t, net, ids) + verifyFull(t, net, ids) }) } } diff --git a/p2p/simulations/test.go b/p2p/simulations/test.go index beeb414e41..86643e18fe 100644 --- a/p2p/simulations/test.go +++ b/p2p/simulations/test.go @@ -96,24 +96,6 @@ func VerifyChain(t *testing.T, net *Network, ids []enode.ID) { } } -func VerifyFull(t *testing.T, net *Network, ids []enode.ID) { - t.Helper() - n := len(ids) - var connections int - for i, lid := range ids { - for _, rid := range ids[i+1:] { - if net.GetConn(lid, rid) != nil { - connections++ - } - } - } - - want := n * (n - 1) / 2 - if connections != want { - t.Errorf("wrong number of connections, got: %v, want: %v", connections, want) - } -} - func VerifyStar(t *testing.T, net *Network, ids []enode.ID, centerIndex int) { t.Helper() n := len(ids) @@ -132,3 +114,21 @@ func VerifyStar(t *testing.T, net *Network, ids []enode.ID, centerIndex int) { } } } + +func verifyFull(t *testing.T, net *Network, ids []enode.ID) { + t.Helper() + n := len(ids) + var connections int + for i, lid := range ids { + for _, rid := range ids[i+1:] { + if net.GetConn(lid, rid) != nil { + connections++ + } + } + } + + want := n * (n - 1) / 2 + if connections != want { + t.Errorf("wrong number of connections, got: %v, want: %v", connections, want) + } +} diff --git a/swarm/network/simulation/node.go b/swarm/network/simulation/node.go index a48dd96ddf..99c23576ad 100644 --- a/swarm/network/simulation/node.go +++ b/swarm/network/simulation/node.go @@ -117,23 +117,6 @@ func (s *Simulation) AddNodes(count int) (ids []enode.ID, err error) { return ids, nil } -// AddNodesAndConnectFull is a helpper method that combines -// AddNodes and ConnectNodesFull. Only new nodes will be connected. -func (s *Simulation) AddNodesAndConnectFull(count int) (ids []enode.ID, err error) { - if count < 2 { - return nil, errors.New("count of nodes must be at least 2") - } - ids, err = s.AddNodes(count) - if err != nil { - return nil, err - } - err = s.Net.ConnectNodesFull(ids) - if err != nil { - return nil, err - } - return ids, nil -} - // AddNodesAndConnectChain is a helpper method that combines // AddNodes and ConnectNodesChain. The chain will be continued from the last // added node, if there is one in simulation using ConnectToLastNode method. diff --git a/swarm/network/simulation/node_test.go b/swarm/network/simulation/node_test.go index 8da32cf37f..7089386b6f 100644 --- a/swarm/network/simulation/node_test.go +++ b/swarm/network/simulation/node_test.go @@ -218,20 +218,6 @@ func TestAddNodes(t *testing.T) { } } -func TestAddNodesAndConnectFull(t *testing.T) { - sim := New(noopServiceFuncMap) - defer sim.Close() - - n := 12 - - ids, err := sim.AddNodesAndConnectFull(n) - if err != nil { - t.Fatal(err) - } - - simulations.VerifyFull(t, sim.Net, ids) -} - func TestAddNodesAndConnectChain(t *testing.T) { sim := New(noopServiceFuncMap) defer sim.Close() From a899495048041cb6d7b6c7747e1d8544fcd2202b Mon Sep 17 00:00:00 2001 From: Ferenc Szabo Date: Thu, 20 Dec 2018 11:45:17 +0100 Subject: [PATCH 04/18] swarm/network: remove AddNodesAndConnectStar() method The method was used only its own test. --- p2p/simulations/connect_test.go | 4 +-- p2p/simulations/test.go | 38 +++++++++++++-------------- swarm/network/simulation/node.go | 17 ------------ swarm/network/simulation/node_test.go | 12 --------- 4 files changed, 21 insertions(+), 50 deletions(-) diff --git a/p2p/simulations/connect_test.go b/p2p/simulations/connect_test.go index 0029b44115..8d113301be 100644 --- a/p2p/simulations/connect_test.go +++ b/p2p/simulations/connect_test.go @@ -185,7 +185,7 @@ func TestConnectNodesStar(t *testing.T) { t.Fatal(err) } - VerifyStar(t, net, ids, pivotIndex) + verifyStar(t, net, ids, pivotIndex) } func TestConnectNodesStarPivot(t *testing.T) { @@ -201,5 +201,5 @@ func TestConnectNodesStarPivot(t *testing.T) { t.Fatal(err) } - VerifyStar(t, net, ids, pivotIndex) + verifyStar(t, net, ids, pivotIndex) } diff --git a/p2p/simulations/test.go b/p2p/simulations/test.go index 86643e18fe..2a09993565 100644 --- a/p2p/simulations/test.go +++ b/p2p/simulations/test.go @@ -96,25 +96,6 @@ func VerifyChain(t *testing.T, net *Network, ids []enode.ID) { } } -func VerifyStar(t *testing.T, net *Network, ids []enode.ID, centerIndex int) { - t.Helper() - n := len(ids) - for i := 0; i < n; i++ { - for j := i + 1; j < n; j++ { - c := net.GetConn(ids[i], ids[j]) - if i == centerIndex || j == centerIndex { - if c == nil { - t.Errorf("nodes %v and %v are not connected, but they should be", i, j) - } - } else { - if c != nil { - t.Errorf("nodes %v and %v are connected, but they should not be", i, j) - } - } - } - } -} - func verifyFull(t *testing.T, net *Network, ids []enode.ID) { t.Helper() n := len(ids) @@ -132,3 +113,22 @@ func verifyFull(t *testing.T, net *Network, ids []enode.ID) { t.Errorf("wrong number of connections, got: %v, want: %v", connections, want) } } + +func verifyStar(t *testing.T, net *Network, ids []enode.ID, centerIndex int) { + t.Helper() + n := len(ids) + for i := 0; i < n; i++ { + for j := i + 1; j < n; j++ { + c := net.GetConn(ids[i], ids[j]) + if i == centerIndex || j == centerIndex { + if c == nil { + t.Errorf("nodes %v and %v are not connected, but they should be", i, j) + } + } else { + if c != nil { + t.Errorf("nodes %v and %v are connected, but they should not be", i, j) + } + } + } + } +} diff --git a/swarm/network/simulation/node.go b/swarm/network/simulation/node.go index 99c23576ad..fe758d7666 100644 --- a/swarm/network/simulation/node.go +++ b/swarm/network/simulation/node.go @@ -161,23 +161,6 @@ func (s *Simulation) AddNodesAndConnectRing(count int) (ids []enode.ID, err erro return ids, nil } -// AddNodesAndConnectStar is a helpper method that combines -// AddNodes and ConnectNodesStar. -func (s *Simulation) AddNodesAndConnectStar(count int) (ids []enode.ID, err error) { - if count < 2 { - return nil, errors.New("count of nodes must be at least 2") - } - ids, err = s.AddNodes(count) - if err != nil { - return nil, err - } - err = s.Net.ConnectNodesStar(ids[0], ids[1:]) - if err != nil { - return nil, err - } - return ids, nil -} - //UploadSnapshot uploads a snapshot to the simulation //This method tries to open the json file provided, applies the config to all nodes //and then loads the snapshot into the Simulation network diff --git a/swarm/network/simulation/node_test.go b/swarm/network/simulation/node_test.go index 7089386b6f..43c681b9e6 100644 --- a/swarm/network/simulation/node_test.go +++ b/swarm/network/simulation/node_test.go @@ -249,18 +249,6 @@ func TestAddNodesAndConnectRing(t *testing.T) { simulations.VerifyRing(t, sim.Net, ids) } -func TestAddNodesAndConnectStar(t *testing.T) { - sim := New(noopServiceFuncMap) - defer sim.Close() - - ids, err := sim.AddNodesAndConnectStar(12) - if err != nil { - t.Fatal(err) - } - - simulations.VerifyStar(t, sim.Net, ids, 0) -} - //To test that uploading a snapshot works func TestUploadSnapshot(t *testing.T) { log.Debug("Creating simulation") From 248b46ed896a79e7c6dca6383ccb99e862d70e4c Mon Sep 17 00:00:00 2001 From: Ferenc Szabo Date: Thu, 20 Dec 2018 14:45:25 +0100 Subject: [PATCH 05/18] p2p/simulations: remove everything related to pivot from connect.go As the concept and the related functionality was not used anywhere. --- p2p/simulations/connect.go | 51 +++------------------------------ p2p/simulations/connect_test.go | 36 +---------------------- p2p/simulations/network.go | 2 -- 3 files changed, 5 insertions(+), 84 deletions(-) diff --git a/p2p/simulations/connect.go b/p2p/simulations/connect.go index 606dda0567..9d32f9e3cc 100644 --- a/p2p/simulations/connect.go +++ b/p2p/simulations/connect.go @@ -28,18 +28,6 @@ var ( 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 +103,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 +131,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 8d113301be..03962176fd 100644 --- a/p2p/simulations/connect_test.go +++ b/p2p/simulations/connect_test.go @@ -57,24 +57,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() @@ -180,23 +162,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 a6fac2c2af..237ad9e4d3 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 From 9d76ab036fadf4070af79a8cc05a10a6e70a0aeb Mon Sep 17 00:00:00 2001 From: Ferenc Szabo Date: Thu, 20 Dec 2018 14:53:27 +0100 Subject: [PATCH 06/18] p2p/simulations: remove unused Err and private method --- p2p/simulations/adapters/inproc.go | 14 -------------- p2p/simulations/connect.go | 1 - 2 files changed, 15 deletions(-) diff --git a/p2p/simulations/adapters/inproc.go b/p2p/simulations/adapters/inproc.go index 52a662be63..0ebc66b2d7 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 9d32f9e3cc..bb7e7999a1 100644 --- a/p2p/simulations/connect.go +++ b/p2p/simulations/connect.go @@ -25,7 +25,6 @@ import ( var ( ErrNodeNotFound = errors.New("node not found") - ErrNoPivotNode = errors.New("no pivot node set") ) // ConnectToLastNode connects the node with provided NodeID From 28cdb50002bd8ae5097c58ec8f49509f15eb3b9a Mon Sep 17 00:00:00 2001 From: Ferenc Szabo Date: Thu, 20 Dec 2018 15:06:41 +0100 Subject: [PATCH 07/18] swarm/network: remove everything related to pivot from Simulation As the concept and the related functionality was not used anywhere. --- swarm/network/simulation/node.go | 19 ------------- swarm/network/simulation/node_test.go | 39 -------------------------- swarm/network/simulation/simulation.go | 1 - 3 files changed, 59 deletions(-) diff --git a/swarm/network/simulation/node.go b/swarm/network/simulation/node.go index fe758d7666..748e369ddb 100644 --- a/swarm/network/simulation/node.go +++ b/swarm/network/simulation/node.go @@ -204,25 +204,6 @@ func (s *Simulation) UploadSnapshot(snapshotFile string) error { 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 43c681b9e6..e158361a0c 100644 --- a/swarm/network/simulation/node_test.go +++ b/swarm/network/simulation/node_test.go @@ -288,45 +288,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 5f39b59117..bfa4498ee7 100644 --- a/swarm/network/simulation/simulation.go +++ b/swarm/network/simulation/simulation.go @@ -45,7 +45,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 From 7c8b0b88976f53c7a9b9654ec59d163373b805ca Mon Sep 17 00:00:00 2001 From: Ferenc Szabo Date: Thu, 20 Dec 2018 15:50:53 +0100 Subject: [PATCH 08/18] swarm/network: remove unused Start/StopRandomNode() --- swarm/network/simulation/node.go | 18 ---------- swarm/network/simulation/node_test.go | 48 --------------------------- 2 files changed, 66 deletions(-) diff --git a/swarm/network/simulation/node.go b/swarm/network/simulation/node.go index 748e369ddb..5733ae0484 100644 --- a/swarm/network/simulation/node.go +++ b/swarm/network/simulation/node.go @@ -209,15 +209,6 @@ func (s *Simulation) StartNode(id enode.ID) (err error) { return s.Net.Start(id) } -// StartRandomNode starts a random node. -func (s *Simulation) StartRandomNode() (id enode.ID, err error) { - n := s.Net.GetRandomDownNode() - if n == nil { - return id, ErrNodeNotFound - } - return n.ID(), s.Net.Start(n.ID()) -} - // StartRandomNodes starts random nodes. func (s *Simulation) StartRandomNodes(count int) (ids []enode.ID, err error) { ids = make([]enode.ID, 0, count) @@ -240,15 +231,6 @@ func (s *Simulation) StopNode(id enode.ID) (err error) { return s.Net.Stop(id) } -// StopRandomNode stops a random node. -func (s *Simulation) StopRandomNode() (id enode.ID, err error) { - n := s.Net.GetRandomUpNode() - if n == nil { - return id, ErrNodeNotFound - } - return n.ID(), s.Net.Stop(n.ID()) -} - // StopRandomNodes stops random nodes. func (s *Simulation) StopRandomNodes(count int) (ids []enode.ID, err error) { ids = make([]enode.ID, 0, count) diff --git a/swarm/network/simulation/node_test.go b/swarm/network/simulation/node_test.go index e158361a0c..cb183ca6ce 100644 --- a/swarm/network/simulation/node_test.go +++ b/swarm/network/simulation/node_test.go @@ -333,54 +333,6 @@ func TestStartStopNode(t *testing.T) { } } -func TestStartStopRandomNode(t *testing.T) { - sim := New(noopServiceFuncMap) - defer sim.Close() - - _, err := sim.AddNodes(3) - if err != nil { - t.Fatal(err) - } - - id, err := sim.StopRandomNode() - if err != nil { - t.Fatal(err) - } - - n := sim.Net.GetNode(id) - if n == nil { - t.Fatal("node not found") - } - if n.Up { - t.Error("node not stopped") - } - - id2, err := sim.StopRandomNode() - if err != nil { - t.Fatal(err) - } - - // Sleep here to ensure that Network.watchPeerEvents defer function - // has set the `node.Up = false` before we start the node again. - // p2p/simulations/network.go:215 - // - // The same node is stopped and started again, and upon start - // watchPeerEvents is started in a goroutine. If the node is stopped - // and then very quickly started, that goroutine may be scheduled later - // then start and force `node.Up = false` in its defer function. - // This will make this test unreliable. - time.Sleep(time.Second) - - idStarted, err := sim.StartRandomNode() - if err != nil { - t.Fatal(err) - } - - if idStarted != id && idStarted != id2 { - t.Error("unexpected started node ID") - } -} - func TestStartStopRandomNodes(t *testing.T) { sim := New(noopServiceFuncMap) defer sim.Close() From 9a6ea0d668c7ae993baa12ed14fd77214cf34327 Mon Sep 17 00:00:00 2001 From: Ferenc Szabo Date: Thu, 20 Dec 2018 16:32:29 +0100 Subject: [PATCH 09/18] swarm/network: remove unused AddNodeWithMsgEvents() --- swarm/network/simulation/node.go | 8 -------- swarm/network/simulation/node_test.go | 23 ----------------------- 2 files changed, 31 deletions(-) diff --git a/swarm/network/simulation/node.go b/swarm/network/simulation/node.go index 5733ae0484..9ec4b388f9 100644 --- a/swarm/network/simulation/node.go +++ b/swarm/network/simulation/node.go @@ -66,14 +66,6 @@ func (s *Simulation) DownNodeIDs() (ids []enode.ID) { // to Simulation.AddNode method. type AddNodeOption func(*adapters.NodeConfig) -// AddNodeWithMsgEvents sets the EnableMsgEvents option -// to NodeConfig. -func AddNodeWithMsgEvents(enable bool) AddNodeOption { - return func(o *adapters.NodeConfig) { - o.EnableMsgEvents = enable - } -} - // AddNodeWithService specifies a service that should be // started on a node. This option can be repeated as variadic // argument toe AddNode and other add node related methods. diff --git a/swarm/network/simulation/node_test.go b/swarm/network/simulation/node_test.go index cb183ca6ce..abd5c58940 100644 --- a/swarm/network/simulation/node_test.go +++ b/swarm/network/simulation/node_test.go @@ -117,29 +117,6 @@ func TestAddNode(t *testing.T) { } } -func TestAddNodeWithMsgEvents(t *testing.T) { - sim := New(noopServiceFuncMap) - defer sim.Close() - - id, err := sim.AddNode(AddNodeWithMsgEvents(true)) - if err != nil { - t.Fatal(err) - } - - if !sim.Net.GetNode(id).Config.EnableMsgEvents { - t.Error("EnableMsgEvents is false") - } - - id, err = sim.AddNode(AddNodeWithMsgEvents(false)) - if err != nil { - t.Fatal(err) - } - - if sim.Net.GetNode(id).Config.EnableMsgEvents { - t.Error("EnableMsgEvents is true") - } -} - func TestAddNodeWithService(t *testing.T) { sim := New(map[string]ServiceFunc{ "noop1": noopServiceFunc, From 242cd7b3c3708cdf9ce4ad24e136c30a3c1a9f11 Mon Sep 17 00:00:00 2001 From: Ferenc Szabo Date: Thu, 20 Dec 2018 16:36:35 +0100 Subject: [PATCH 10/18] swarm/network: remove unused AddNodeWithService() That also makes AddNodeOption type and the AddNode's AddNodeOptions parameter unnecessary. --- swarm/network/simulation/node.go | 25 +++---------------------- swarm/network/simulation/node_test.go | 21 --------------------- 2 files changed, 3 insertions(+), 43 deletions(-) diff --git a/swarm/network/simulation/node.go b/swarm/network/simulation/node.go index 9ec4b388f9..0ab4348696 100644 --- a/swarm/network/simulation/node.go +++ b/swarm/network/simulation/node.go @@ -62,29 +62,10 @@ func (s *Simulation) DownNodeIDs() (ids []enode.ID) { return ids } -// AddNodeOption defines the option that can be passed -// to Simulation.AddNode method. -type AddNodeOption func(*adapters.NodeConfig) - -// AddNodeWithService specifies a service that should be -// started on a node. This option can be repeated as variadic -// argument toe AddNode and other add node related methods. -// If AddNodeWithService is not specified, all services will be started. -func AddNodeWithService(serviceName string) AddNodeOption { - return func(o *adapters.NodeConfig) { - o.Services = append(o.Services, serviceName) - } -} - -// AddNode creates a new node with random configuration, -// applies provided options to the config and adds the node to network. -// By default all services will be started on a node. If one or more -// AddNodeWithService option are provided, only specified services will be started. -func (s *Simulation) AddNode(opts ...AddNodeOption) (id enode.ID, err error) { +// AddNode creates a new node with random configuration and adds that to +// the network. All services will be started on the node. +func (s *Simulation) AddNode() (id enode.ID, err error) { conf := adapters.RandomNodeConfig() - for _, o := range opts { - o(conf) - } if len(conf.Services) == 0 { conf.Services = s.serviceNames } diff --git a/swarm/network/simulation/node_test.go b/swarm/network/simulation/node_test.go index abd5c58940..48c4f06069 100644 --- a/swarm/network/simulation/node_test.go +++ b/swarm/network/simulation/node_test.go @@ -117,27 +117,6 @@ func TestAddNode(t *testing.T) { } } -func TestAddNodeWithService(t *testing.T) { - sim := New(map[string]ServiceFunc{ - "noop1": noopServiceFunc, - "noop2": noopServiceFunc, - }) - defer sim.Close() - - id, err := sim.AddNode(AddNodeWithService("noop1")) - if err != nil { - t.Fatal(err) - } - - n := sim.Net.GetNode(id).Node.(*adapters.SimNode) - if n.Service("noop1") == nil { - t.Error("service noop1 not found on node") - } - if n.Service("noop2") != nil { - t.Error("service noop2 should not be found on node") - } -} - func TestAddNodeMultipleServices(t *testing.T) { sim := New(map[string]ServiceFunc{ "noop1": noopServiceFunc, From 2808be28ee9dd837634253a1faf7ced4b65c7e43 Mon Sep 17 00:00:00 2001 From: Ferenc Szabo Date: Thu, 20 Dec 2018 17:18:53 +0100 Subject: [PATCH 11/18] swarm/network: remove unused Start/StopNode() As we can just call Stop() on Network. --- swarm/network/simulation/bucket_test.go | 2 +- swarm/network/simulation/node.go | 10 ------ swarm/network/simulation/node_test.go | 45 ------------------------- 3 files changed, 1 insertion(+), 56 deletions(-) diff --git a/swarm/network/simulation/bucket_test.go b/swarm/network/simulation/bucket_test.go index 69df19bfe4..dbe3e93a6a 100644 --- a/swarm/network/simulation/bucket_test.go +++ b/swarm/network/simulation/bucket_test.go @@ -100,7 +100,7 @@ func TestServiceBucket(t *testing.T) { } }) - if err := sim.StopNode(id2); err != nil { + if err := sim.Net.Stop(id2); err != nil { t.Fatal(err) } diff --git a/swarm/network/simulation/node.go b/swarm/network/simulation/node.go index 0ab4348696..818cb7bd85 100644 --- a/swarm/network/simulation/node.go +++ b/swarm/network/simulation/node.go @@ -177,11 +177,6 @@ func (s *Simulation) UploadSnapshot(snapshotFile string) error { return nil } -// StartNode starts a node by NodeID. -func (s *Simulation) StartNode(id enode.ID) (err error) { - return s.Net.Start(id) -} - // StartRandomNodes starts random nodes. func (s *Simulation) StartRandomNodes(count int) (ids []enode.ID, err error) { ids = make([]enode.ID, 0, count) @@ -199,11 +194,6 @@ func (s *Simulation) StartRandomNodes(count int) (ids []enode.ID, err error) { return ids, nil } -// StopNode stops a node by NodeID. -func (s *Simulation) StopNode(id enode.ID) (err error) { - return s.Net.Stop(id) -} - // StopRandomNodes stops random nodes. func (s *Simulation) StopRandomNodes(count int) (ids []enode.ID, err error) { ids = make([]enode.ID, 0, count) diff --git a/swarm/network/simulation/node_test.go b/swarm/network/simulation/node_test.go index 48c4f06069..a715ba9bc7 100644 --- a/swarm/network/simulation/node_test.go +++ b/swarm/network/simulation/node_test.go @@ -244,51 +244,6 @@ func TestUploadSnapshot(t *testing.T) { log.Debug("Done.") } -func TestStartStopNode(t *testing.T) { - sim := New(noopServiceFuncMap) - defer sim.Close() - - id, err := sim.AddNode() - if err != nil { - t.Fatal(err) - } - - n := sim.Net.GetNode(id) - if n == nil { - t.Fatal("node not found") - } - if !n.Up { - t.Error("node not started") - } - - err = sim.StopNode(id) - if err != nil { - t.Fatal(err) - } - if n.Up { - t.Error("node not stopped") - } - - // Sleep here to ensure that Network.watchPeerEvents defer function - // has set the `node.Up = false` before we start the node again. - // p2p/simulations/network.go:215 - // - // The same node is stopped and started again, and upon start - // watchPeerEvents is started in a goroutine. If the node is stopped - // and then very quickly started, that goroutine may be scheduled later - // then start and force `node.Up = false` in its defer function. - // This will make this test unreliable. - time.Sleep(time.Second) - - err = sim.StartNode(id) - if err != nil { - t.Fatal(err) - } - if !n.Up { - t.Error("node not started") - } -} - func TestStartStopRandomNodes(t *testing.T) { sim := New(noopServiceFuncMap) defer sim.Close() From 5decff8d5333b2b511a75cc1da3cdcdd06816152 Mon Sep 17 00:00:00 2001 From: Ferenc Szabo Date: Fri, 21 Dec 2018 12:57:05 +0100 Subject: [PATCH 12/18] swarm/network: remove AddNodesAndConnectRing() method The utility of this function was very low. The method just combined two simple calls into one and was used in only one place. Note: verifyRing() now can be private. --- p2p/simulations/connect_test.go | 2 +- p2p/simulations/test.go | 38 +++++++++++------------ swarm/network/simulation/kademlia_test.go | 6 +++- swarm/network/simulation/node.go | 17 ---------- swarm/network/simulation/node_test.go | 12 ------- 5 files changed, 25 insertions(+), 50 deletions(-) diff --git a/p2p/simulations/connect_test.go b/p2p/simulations/connect_test.go index 03962176fd..f40fa335d3 100644 --- a/p2p/simulations/connect_test.go +++ b/p2p/simulations/connect_test.go @@ -153,7 +153,7 @@ func TestConnectNodesRing(t *testing.T) { t.Fatal(err) } - VerifyRing(t, net, ids) + verifyRing(t, net, ids) } func TestConnectNodesStar(t *testing.T) { diff --git a/p2p/simulations/test.go b/p2p/simulations/test.go index 2a09993565..7c2ad0b3ac 100644 --- a/p2p/simulations/test.go +++ b/p2p/simulations/test.go @@ -58,25 +58,6 @@ func (t *NoopService) Stop() error { return nil } -func VerifyRing(t *testing.T, net *Network, ids []enode.ID) { - t.Helper() - n := len(ids) - for i := 0; i < n; i++ { - for j := i + 1; j < n; j++ { - c := net.GetConn(ids[i], ids[j]) - if i == j-1 || (i == 0 && j == n-1) { - if c == nil { - t.Errorf("nodes %v and %v are not connected, but they should be", i, j) - } - } else { - if c != nil { - t.Errorf("nodes %v and %v are connected, but they should not be", i, j) - } - } - } - } -} - func VerifyChain(t *testing.T, net *Network, ids []enode.ID) { t.Helper() n := len(ids) @@ -114,6 +95,25 @@ func verifyFull(t *testing.T, net *Network, ids []enode.ID) { } } +func verifyRing(t *testing.T, net *Network, ids []enode.ID) { + t.Helper() + n := len(ids) + for i := 0; i < n; i++ { + for j := i + 1; j < n; j++ { + c := net.GetConn(ids[i], ids[j]) + if i == j-1 || (i == 0 && j == n-1) { + if c == nil { + t.Errorf("nodes %v and %v are not connected, but they should be", i, j) + } + } else { + if c != nil { + t.Errorf("nodes %v and %v are connected, but they should not be", i, j) + } + } + } + } +} + func verifyStar(t *testing.T, net *Network, ids []enode.ID, centerIndex int) { t.Helper() n := len(ids) diff --git a/swarm/network/simulation/kademlia_test.go b/swarm/network/simulation/kademlia_test.go index f02b0e5417..52e09c5fc9 100644 --- a/swarm/network/simulation/kademlia_test.go +++ b/swarm/network/simulation/kademlia_test.go @@ -47,11 +47,15 @@ func TestWaitTillHealthy(t *testing.T) { }) defer sim.Close() - _, err := sim.AddNodesAndConnectRing(10) + ids, err := sim.AddNodes(10) if err != nil { t.Fatal(err) } + if err := sim.Net.ConnectNodesRing(ids); err != nil { + t.Fatal(err) + } + ctx, cancel := context.WithTimeout(context.Background(), 120*time.Second) defer cancel() ill, err := sim.WaitTillHealthy(ctx, 2) diff --git a/swarm/network/simulation/node.go b/swarm/network/simulation/node.go index 818cb7bd85..a73369617d 100644 --- a/swarm/network/simulation/node.go +++ b/swarm/network/simulation/node.go @@ -117,23 +117,6 @@ func (s *Simulation) AddNodesAndConnectChain(count int) (ids []enode.ID, err err return ids, nil } -// AddNodesAndConnectRing is a helpper method that combines -// AddNodes and ConnectNodesRing. -func (s *Simulation) AddNodesAndConnectRing(count int) (ids []enode.ID, err error) { - if count < 2 { - return nil, errors.New("count of nodes must be at least 2") - } - ids, err = s.AddNodes(count) - if err != nil { - return nil, err - } - err = s.Net.ConnectNodesRing(ids) - if err != nil { - return nil, err - } - return ids, nil -} - //UploadSnapshot uploads a snapshot to the simulation //This method tries to open the json file provided, applies the config to all nodes //and then loads the snapshot into the Simulation network diff --git a/swarm/network/simulation/node_test.go b/swarm/network/simulation/node_test.go index a715ba9bc7..d65d8f37a3 100644 --- a/swarm/network/simulation/node_test.go +++ b/swarm/network/simulation/node_test.go @@ -193,18 +193,6 @@ func TestAddNodesAndConnectChain(t *testing.T) { simulations.VerifyChain(t, sim.Net, sim.UpNodeIDs()) } -func TestAddNodesAndConnectRing(t *testing.T) { - sim := New(noopServiceFuncMap) - defer sim.Close() - - ids, err := sim.AddNodesAndConnectRing(12) - if err != nil { - t.Fatal(err) - } - - simulations.VerifyRing(t, sim.Net, ids) -} - //To test that uploading a snapshot works func TestUploadSnapshot(t *testing.T) { log.Debug("Creating simulation") From fa98d4e10ad9d0961af4ed3c3e4fae3881ac8f32 Mon Sep 17 00:00:00 2001 From: Ferenc Szabo Date: Mon, 24 Dec 2018 16:21:08 +0100 Subject: [PATCH 13/18] swarm/network: remove AddNodesAndConnectChain() method This was the lat mixed responsibility function in node.go, i.e., not just adding but connecting nodes. As the responsibility of connecting nodes belongs to p2p/simulations/connect.go I removed the method. Note: there was just one complex use case for AddNodesAndConnectChain in testSwarmNetwork where we actually added nodes more than once and we still wanted to keep them in chain topology. There I introduced a test helper function. --- p2p/simulations/connect_test.go | 77 +++++++++++++++++++++++++- p2p/simulations/test.go | 77 -------------------------- swarm/network/simulation/node.go | 28 ---------- swarm/network/simulation/node_test.go | 20 ------- swarm/network/stream/delivery_test.go | 30 ++++++---- swarm/network/stream/intervals_test.go | 6 +- swarm/network/stream/syncer_test.go | 19 +++++-- swarm/network_test.go | 33 +++++++++-- 8 files changed, 141 insertions(+), 149 deletions(-) diff --git a/p2p/simulations/connect_test.go b/p2p/simulations/connect_test.go index f40fa335d3..3da017b158 100644 --- a/p2p/simulations/connect_test.go +++ b/p2p/simulations/connect_test.go @@ -132,6 +132,24 @@ func TestConnectNodesFull(t *testing.T) { } } +func verifyFull(t *testing.T, net *Network, ids []enode.ID) { + t.Helper() + n := len(ids) + var connections int + for i, lid := range ids { + for _, rid := range ids[i+1:] { + if net.GetConn(lid, rid) != nil { + connections++ + } + } + } + + want := n * (n - 1) / 2 + if connections != want { + t.Errorf("wrong number of connections, got: %v, want: %v", connections, want) + } +} + func TestConnectNodesChain(t *testing.T) { net, ids := newTestNetwork(t, 10) defer net.Shutdown() @@ -141,7 +159,26 @@ func TestConnectNodesChain(t *testing.T) { t.Fatal(err) } - VerifyChain(t, net, ids) + verifyChain(t, net, ids) +} + +func verifyChain(t *testing.T, net *Network, ids []enode.ID) { + t.Helper() + n := len(ids) + for i := 0; i < n; i++ { + for j := i + 1; j < n; j++ { + c := net.GetConn(ids[i], ids[j]) + if i == j-1 { + if c == nil { + t.Errorf("nodes %v and %v are not connected, but they should be", i, j) + } + } else { + if c != nil { + t.Errorf("nodes %v and %v are connected, but they should not be", i, j) + } + } + } + } } func TestConnectNodesRing(t *testing.T) { @@ -156,6 +193,25 @@ func TestConnectNodesRing(t *testing.T) { verifyRing(t, net, ids) } +func verifyRing(t *testing.T, net *Network, ids []enode.ID) { + t.Helper() + n := len(ids) + for i := 0; i < n; i++ { + for j := i + 1; j < n; j++ { + c := net.GetConn(ids[i], ids[j]) + if i == j-1 || (i == 0 && j == n-1) { + if c == nil { + t.Errorf("nodes %v and %v are not connected, but they should be", i, j) + } + } else { + if c != nil { + t.Errorf("nodes %v and %v are connected, but they should not be", i, j) + } + } + } + } +} + func TestConnectNodesStar(t *testing.T) { net, ids := newTestNetwork(t, 10) defer net.Shutdown() @@ -169,3 +225,22 @@ func TestConnectNodesStar(t *testing.T) { verifyStar(t, net, ids, pivotIndex) } + +func verifyStar(t *testing.T, net *Network, ids []enode.ID, centerIndex int) { + t.Helper() + n := len(ids) + for i := 0; i < n; i++ { + for j := i + 1; j < n; j++ { + c := net.GetConn(ids[i], ids[j]) + if i == centerIndex || j == centerIndex { + if c == nil { + t.Errorf("nodes %v and %v are not connected, but they should be", i, j) + } + } else { + if c != nil { + t.Errorf("nodes %v and %v are connected, but they should not be", i, j) + } + } + } + } +} diff --git a/p2p/simulations/test.go b/p2p/simulations/test.go index 7c2ad0b3ac..6a646f9ecb 100644 --- a/p2p/simulations/test.go +++ b/p2p/simulations/test.go @@ -1,8 +1,6 @@ package simulations import ( - "testing" - "github.com/ethereum/go-ethereum/p2p" "github.com/ethereum/go-ethereum/p2p/enode" "github.com/ethereum/go-ethereum/p2p/enr" @@ -57,78 +55,3 @@ func (t *NoopService) Start(server *p2p.Server) error { func (t *NoopService) Stop() error { return nil } - -func VerifyChain(t *testing.T, net *Network, ids []enode.ID) { - t.Helper() - n := len(ids) - for i := 0; i < n; i++ { - for j := i + 1; j < n; j++ { - c := net.GetConn(ids[i], ids[j]) - if i == j-1 { - if c == nil { - t.Errorf("nodes %v and %v are not connected, but they should be", i, j) - } - } else { - if c != nil { - t.Errorf("nodes %v and %v are connected, but they should not be", i, j) - } - } - } - } -} - -func verifyFull(t *testing.T, net *Network, ids []enode.ID) { - t.Helper() - n := len(ids) - var connections int - for i, lid := range ids { - for _, rid := range ids[i+1:] { - if net.GetConn(lid, rid) != nil { - connections++ - } - } - } - - want := n * (n - 1) / 2 - if connections != want { - t.Errorf("wrong number of connections, got: %v, want: %v", connections, want) - } -} - -func verifyRing(t *testing.T, net *Network, ids []enode.ID) { - t.Helper() - n := len(ids) - for i := 0; i < n; i++ { - for j := i + 1; j < n; j++ { - c := net.GetConn(ids[i], ids[j]) - if i == j-1 || (i == 0 && j == n-1) { - if c == nil { - t.Errorf("nodes %v and %v are not connected, but they should be", i, j) - } - } else { - if c != nil { - t.Errorf("nodes %v and %v are connected, but they should not be", i, j) - } - } - } - } -} - -func verifyStar(t *testing.T, net *Network, ids []enode.ID, centerIndex int) { - t.Helper() - n := len(ids) - for i := 0; i < n; i++ { - for j := i + 1; j < n; j++ { - c := net.GetConn(ids[i], ids[j]) - if i == centerIndex || j == centerIndex { - if c == nil { - t.Errorf("nodes %v and %v are not connected, but they should be", i, j) - } - } else { - if c != nil { - t.Errorf("nodes %v and %v are connected, but they should not be", i, j) - } - } - } - } -} diff --git a/swarm/network/simulation/node.go b/swarm/network/simulation/node.go index a73369617d..110455c1ad 100644 --- a/swarm/network/simulation/node.go +++ b/swarm/network/simulation/node.go @@ -18,7 +18,6 @@ package simulation import ( "encoding/json" - "errors" "io/ioutil" "math/rand" "os" @@ -90,33 +89,6 @@ func (s *Simulation) AddNodes(count int) (ids []enode.ID, err error) { return ids, nil } -// AddNodesAndConnectChain is a helpper method that combines -// AddNodes and ConnectNodesChain. The chain will be continued from the last -// added node, if there is one in simulation using ConnectToLastNode method. -func (s *Simulation) AddNodesAndConnectChain(count int) (ids []enode.ID, err error) { - if count < 2 { - return nil, errors.New("count of nodes must be at least 2") - } - id, err := s.AddNode() - if err != nil { - return nil, err - } - err = s.Net.ConnectToLastNode(id) - if err != nil { - return nil, err - } - ids, err = s.AddNodes(count - 1) - if err != nil { - return nil, err - } - ids = append([]enode.ID{id}, ids...) - err = s.Net.ConnectNodesChain(ids) - if err != nil { - return nil, err - } - return ids, nil -} - //UploadSnapshot uploads a snapshot to the simulation //This method tries to open the json file provided, applies the config to all nodes //and then loads the snapshot into the Simulation network diff --git a/swarm/network/simulation/node_test.go b/swarm/network/simulation/node_test.go index d65d8f37a3..eb4cf48569 100644 --- a/swarm/network/simulation/node_test.go +++ b/swarm/network/simulation/node_test.go @@ -26,7 +26,6 @@ import ( "github.com/ethereum/go-ethereum/log" "github.com/ethereum/go-ethereum/node" "github.com/ethereum/go-ethereum/p2p/enode" - "github.com/ethereum/go-ethereum/p2p/simulations" "github.com/ethereum/go-ethereum/p2p/simulations/adapters" "github.com/ethereum/go-ethereum/swarm/network" ) @@ -174,25 +173,6 @@ func TestAddNodes(t *testing.T) { } } -func TestAddNodesAndConnectChain(t *testing.T) { - sim := New(noopServiceFuncMap) - defer sim.Close() - - _, err := sim.AddNodesAndConnectChain(12) - if err != nil { - t.Fatal(err) - } - - // add another set of nodes to test - // if two chains are connected - _, err = sim.AddNodesAndConnectChain(7) - if err != nil { - t.Fatal(err) - } - - simulations.VerifyChain(t, sim.Net, sim.UpNodeIDs()) -} - //To test that uploading a snapshot works func TestUploadSnapshot(t *testing.T) { log.Debug("Creating simulation") diff --git a/swarm/network/stream/delivery_test.go b/swarm/network/stream/delivery_test.go index 5c1f8c2512..2a7c072c41 100644 --- a/swarm/network/stream/delivery_test.go +++ b/swarm/network/stream/delivery_test.go @@ -442,17 +442,17 @@ func TestStreamerDownstreamChunkDeliveryMsgExchange(t *testing.T) { } func TestDeliveryFromNodes(t *testing.T) { - testDeliveryFromNodes(t, 2, 1, dataChunkCount, true) - testDeliveryFromNodes(t, 2, 1, dataChunkCount, false) - testDeliveryFromNodes(t, 4, 1, dataChunkCount, true) - testDeliveryFromNodes(t, 4, 1, dataChunkCount, false) - testDeliveryFromNodes(t, 8, 1, dataChunkCount, true) - testDeliveryFromNodes(t, 8, 1, dataChunkCount, false) - testDeliveryFromNodes(t, 16, 1, dataChunkCount, true) - testDeliveryFromNodes(t, 16, 1, dataChunkCount, false) + testDeliveryFromNodes(t, 2, dataChunkCount, true) + testDeliveryFromNodes(t, 2, dataChunkCount, false) + testDeliveryFromNodes(t, 4, dataChunkCount, true) + testDeliveryFromNodes(t, 4, dataChunkCount, false) + testDeliveryFromNodes(t, 8, dataChunkCount, true) + testDeliveryFromNodes(t, 8, dataChunkCount, false) + testDeliveryFromNodes(t, 16, dataChunkCount, true) + testDeliveryFromNodes(t, 16, dataChunkCount, false) } -func testDeliveryFromNodes(t *testing.T, nodes, conns, chunkCount int, skipCheck bool) { +func testDeliveryFromNodes(t *testing.T, numberOfNodes, chunkCount int, skipCheck bool) { sim := simulation.New(map[string]simulation.ServiceFunc{ "streamer": func(ctx *adapters.ServiceContext, bucket *sync.Map) (s node.Service, cleanup func(), err error) { node := ctx.Config.Node() @@ -493,10 +493,13 @@ func testDeliveryFromNodes(t *testing.T, nodes, conns, chunkCount int, skipCheck defer sim.Close() log.Info("Adding nodes to simulation") - _, err := sim.AddNodesAndConnectChain(nodes) + ids, err := sim.AddNodes(numberOfNodes) if err != nil { t.Fatal(err) } + if err := sim.Net.ConnectNodesChain(ids); err != nil { + t.Fatal(err) + } log.Info("Starting simulation") ctx := context.Background() @@ -629,7 +632,7 @@ func BenchmarkDeliveryFromNodesWithCheck(b *testing.B) { } } -func benchmarkDeliveryFromNodes(b *testing.B, nodes, conns, chunkCount int, skipCheck bool) { +func benchmarkDeliveryFromNodes(b *testing.B, numberOfNodes, conns, chunkCount int, skipCheck bool) { sim := simulation.New(map[string]simulation.ServiceFunc{ "streamer": func(ctx *adapters.ServiceContext, bucket *sync.Map) (s node.Service, cleanup func(), err error) { node := ctx.Config.Node() @@ -669,10 +672,13 @@ func benchmarkDeliveryFromNodes(b *testing.B, nodes, conns, chunkCount int, skip defer sim.Close() log.Info("Initializing test config") - _, err := sim.AddNodesAndConnectChain(nodes) + ids, err := sim.AddNodes(numberOfNodes) if err != nil { b.Fatal(err) } + if err := sim.Net.ConnectNodesChain(ids); err != nil { + b.Fatal(err) + } ctx := context.Background() result := sim.Run(ctx, func(ctx context.Context, sim *simulation.Simulation) error { diff --git a/swarm/network/stream/intervals_test.go b/swarm/network/stream/intervals_test.go index 7c7feeb112..027890d3dc 100644 --- a/swarm/network/stream/intervals_test.go +++ b/swarm/network/stream/intervals_test.go @@ -53,7 +53,6 @@ func TestIntervalsLiveAndHistory(t *testing.T) { func testIntervals(t *testing.T, live bool, history *Range, skipCheck bool) { - nodes := 2 chunkCount := dataChunkCount externalStreamName := "externalStream" externalStreamSessionAt := uint64(50) @@ -105,10 +104,13 @@ func testIntervals(t *testing.T, live bool, history *Range, skipCheck bool) { defer sim.Close() log.Info("Adding nodes to simulation") - _, err := sim.AddNodesAndConnectChain(nodes) + ids, err := sim.AddNodes(2) if err != nil { t.Fatal(err) } + if err := sim.Net.ConnectNodesChain(ids); err != nil { + t.Fatal(err) + } ctx, cancel := context.WithTimeout(context.Background(), 20*time.Second) defer cancel() diff --git a/swarm/network/stream/syncer_test.go b/swarm/network/stream/syncer_test.go index 27ed49ea48..509c5d252b 100644 --- a/swarm/network/stream/syncer_test.go +++ b/swarm/network/stream/syncer_test.go @@ -120,15 +120,18 @@ func testSyncBetweenNodes(t *testing.T, nodes, conns, chunkCount int, skipCheck defer sim.Close() // create context for simulation run - timeout := 30 * time.Second - ctx, cancel := context.WithTimeout(context.Background(), timeout) + ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) // defer cancel should come before defer simulation teardown defer cancel() - _, err := sim.AddNodesAndConnectChain(nodes) + ids, err := sim.AddNodes(nodes) if err != nil { t.Fatal(err) } + if err := sim.Net.ConnectNodesChain(ids); err != nil { + t.Fatal(err) + } + result := sim.Run(ctx, func(ctx context.Context, sim *simulation.Simulation) error { nodeIDs := sim.UpNodeIDs() @@ -285,10 +288,13 @@ func TestSameVersionID(t *testing.T) { //connect just two nodes log.Info("Adding nodes to simulation") - _, err := sim.AddNodesAndConnectChain(2) + ids, err := sim.AddNodes(2) if err != nil { t.Fatal(err) } + if err := sim.Net.ConnectNodesChain(ids); err != nil { + t.Fatal(err) + } log.Info("Starting simulation") ctx := context.Background() @@ -369,10 +375,13 @@ func TestDifferentVersionID(t *testing.T) { //connect the nodes log.Info("Adding nodes to simulation") - _, err := sim.AddNodesAndConnectChain(2) + ids, err := sim.AddNodes(2) if err != nil { t.Fatal(err) } + if err := sim.Net.ConnectNodesChain(ids); err != nil { + t.Fatal(err) + } log.Info("Starting simulation") ctx := context.Background() diff --git a/swarm/network_test.go b/swarm/network_test.go index 8a162a219e..2e093908dd 100644 --- a/swarm/network_test.go +++ b/swarm/network_test.go @@ -316,10 +316,7 @@ func testSwarmNetwork(t *testing.T, o *testSwarmNetworkOptions, steps ...testSwa change := step.nodeCount - len(sim.UpNodeIDs()) if change > 0 { - _, err := sim.AddNodesAndConnectChain(change) - if err != nil { - t.Fatal(err) - } + addNodesAndPreserveChainTopology(t, sim, change) } else if change < 0 { _, err := sim.StopRandomNodes(-change) if err != nil { @@ -374,6 +371,34 @@ func testSwarmNetwork(t *testing.T, o *testSwarmNetworkOptions, steps ...testSwa } } +func addNodesAndPreserveChainTopology(t *testing.T, s *simulation.Simulation, increment int) { + t.Helper() + + if increment < 1 { + t.Fatalf("change must be positive (increment: %v)", increment) + } + + oldNodeIDs := s.UpNodeIDs() + + newNodeIDs, err := s.AddNodes(increment) + if err != nil { + t.Fatal(err) + } + + if err := s.Net.ConnectNodesChain(newNodeIDs); err != nil { + t.Fatal(err) + } + + // If we had old nodes, then our cluster is split into two now. + // Let's connect the two components and preserve chain topology. + if len(oldNodeIDs) > 0 { + lastOld := oldNodeIDs[len(oldNodeIDs)-1] + if err := s.Net.Connect(lastOld, newNodeIDs[0]); err != nil { + t.Fatal(err) + } + } +} + // uploadFile, uploads a short file to the swarm instance // using the api.Put method. func uploadFile(swarm *Swarm) (storage.Address, string, error) { From fc44737c4f032d93c245c41c62d2c049949e2649 Mon Sep 17 00:00:00 2001 From: Ferenc Szabo Date: Mon, 24 Dec 2018 16:44:06 +0100 Subject: [PATCH 14/18] swarm/network: fix unused method parameters inspections --- swarm/network/simulation/simulation_test.go | 4 ++-- swarm/network/stream/delivery_test.go | 6 +++--- swarm/network/stream/syncer_test.go | 10 +++++----- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/swarm/network/simulation/simulation_test.go b/swarm/network/simulation/simulation_test.go index 4667a2abc3..abbe4826ef 100644 --- a/swarm/network/simulation/simulation_test.go +++ b/swarm/network/simulation/simulation_test.go @@ -177,7 +177,7 @@ var noopServiceFuncMap = map[string]ServiceFunc{ } // a helper function for most basic noop service -func noopServiceFunc(ctx *adapters.ServiceContext, b *sync.Map) (node.Service, func(), error) { +func noopServiceFunc(_ *adapters.ServiceContext, _ *sync.Map) (node.Service, func(), error) { return newNoopService(), nil, nil } @@ -188,7 +188,7 @@ func newNoopService() node.Service { // a helper function for most basic Noop service // of a different type then NoopService to test // multiple services on one node. -func noopService2Func(ctx *adapters.ServiceContext, b *sync.Map) (node.Service, func(), error) { +func noopService2Func(_ *adapters.ServiceContext, _ *sync.Map) (node.Service, func(), error) { return new(noopService2), nil, nil } diff --git a/swarm/network/stream/delivery_test.go b/swarm/network/stream/delivery_test.go index 2a7c072c41..9dd87adce5 100644 --- a/swarm/network/stream/delivery_test.go +++ b/swarm/network/stream/delivery_test.go @@ -612,7 +612,7 @@ func BenchmarkDeliveryFromNodesWithoutCheck(b *testing.B) { b.Run( fmt.Sprintf("nodes=%v,chunks=%v", i, chunks), func(b *testing.B) { - benchmarkDeliveryFromNodes(b, i, 1, chunks, true) + benchmarkDeliveryFromNodes(b, i, chunks, true) }, ) } @@ -625,14 +625,14 @@ func BenchmarkDeliveryFromNodesWithCheck(b *testing.B) { b.Run( fmt.Sprintf("nodes=%v,chunks=%v", i, chunks), func(b *testing.B) { - benchmarkDeliveryFromNodes(b, i, 1, chunks, false) + benchmarkDeliveryFromNodes(b, i, chunks, false) }, ) } } } -func benchmarkDeliveryFromNodes(b *testing.B, numberOfNodes, conns, chunkCount int, skipCheck bool) { +func benchmarkDeliveryFromNodes(b *testing.B, numberOfNodes, chunkCount int, skipCheck bool) { sim := simulation.New(map[string]simulation.ServiceFunc{ "streamer": func(ctx *adapters.ServiceContext, bucket *sync.Map) (s node.Service, cleanup func(), err error) { node := ctx.Config.Node() diff --git a/swarm/network/stream/syncer_test.go b/swarm/network/stream/syncer_test.go index 509c5d252b..ba6ec073c6 100644 --- a/swarm/network/stream/syncer_test.go +++ b/swarm/network/stream/syncer_test.go @@ -43,10 +43,10 @@ import ( const dataChunkCount = 200 func TestSyncerSimulation(t *testing.T) { - testSyncBetweenNodes(t, 2, 1, dataChunkCount, true, 1) - testSyncBetweenNodes(t, 4, 1, dataChunkCount, true, 1) - testSyncBetweenNodes(t, 8, 1, dataChunkCount, true, 1) - testSyncBetweenNodes(t, 16, 1, dataChunkCount, true, 1) + testSyncBetweenNodes(t, 2, dataChunkCount, true, 1) + testSyncBetweenNodes(t, 4, dataChunkCount, true, 1) + testSyncBetweenNodes(t, 8, dataChunkCount, true, 1) + testSyncBetweenNodes(t, 16, dataChunkCount, true, 1) } func createMockStore(globalStore mock.GlobalStorer, id enode.ID, addr *network.BzzAddr) (lstore storage.ChunkStore, datadir string, err error) { @@ -67,7 +67,7 @@ func createMockStore(globalStore mock.GlobalStorer, id enode.ID, addr *network.B return lstore, datadir, nil } -func testSyncBetweenNodes(t *testing.T, nodes, conns, chunkCount int, skipCheck bool, po uint8) { +func testSyncBetweenNodes(t *testing.T, nodes, chunkCount int, skipCheck bool, po uint8) { sim := simulation.New(map[string]simulation.ServiceFunc{ "streamer": func(ctx *adapters.ServiceContext, bucket *sync.Map) (s node.Service, cleanup func(), err error) { From 999f79aa33056444d746d949a5e80e46bafa1517 Mon Sep 17 00:00:00 2001 From: Ferenc Szabo Date: Wed, 26 Dec 2018 12:11:46 +0100 Subject: [PATCH 15/18] p2p/simulations: drop unused Connect* methods from connect.go The deleted methods were all unused. So I don't want to maintain them during refactor. --- p2p/simulations/connect.go | 61 ---------------- p2p/simulations/connect_test.go | 126 -------------------------------- 2 files changed, 187 deletions(-) diff --git a/p2p/simulations/connect.go b/p2p/simulations/connect.go index bb7e7999a1..094de579ff 100644 --- a/p2p/simulations/connect.go +++ b/p2p/simulations/connect.go @@ -27,50 +27,6 @@ var ( ErrNodeNotFound = errors.New("node not found") ) -// 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 -// when Network adds and removes nodes dynamically. -func (net *Network) ConnectToLastNode(id enode.ID) (err error) { - ids := net.getUpNodeIDs() - l := len(ids) - if l < 2 { - return nil - } - last := ids[l-1] - if last == id { - last = ids[l-2] - } - return net.connect(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) - if selected == nil { - return ErrNodeNotFound - } - return net.connect(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) { - 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 { - return err - } - } - } - return nil -} - // 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) { @@ -102,23 +58,6 @@ func (net *Network) ConnectNodesRing(ids []enode.ID) (err error) { return net.connect(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) { - if ids == nil { - ids = net.getUpNodeIDs() - } - for _, id := range ids { - if center == id { - continue - } - if err := net.connect(center, id); err != nil { - return err - } - } - return nil -} - // connect connects two nodes but ignores already connected error. func (net *Network) connect(oneID, otherID enode.ID) error { return ignoreAlreadyConnectedErr(net.Connect(oneID, otherID)) diff --git a/p2p/simulations/connect_test.go b/p2p/simulations/connect_test.go index 3da017b158..88b58b8fa0 100644 --- a/p2p/simulations/connect_test.go +++ b/p2p/simulations/connect_test.go @@ -57,99 +57,6 @@ func newTestNetwork(t *testing.T, nodeCount int) (*Network, []enode.ID) { return network, ids } -func TestConnectToLastNode(t *testing.T) { - net, ids := newTestNetwork(t, 10) - defer net.Shutdown() - - first := ids[0] - if err := net.ConnectToLastNode(first); err != nil { - t.Fatal(err) - } - - last := ids[len(ids)-1] - for i, id := range ids { - if id == first || id == last { - continue - } - - if net.GetConn(first, id) != nil { - t.Errorf("connection must not exist with node(ind: %v, id: %v)", i, id) - } - } - - if net.GetConn(first, last) == nil { - t.Error("first and last node must be connected") - } -} - -func TestConnectToRandomNode(t *testing.T) { - net, ids := newTestNetwork(t, 10) - defer net.Shutdown() - - err := net.ConnectToRandomNode(ids[0]) - if err != nil { - t.Fatal(err) - } - - var cc int - for i, a := range ids { - for _, b := range ids[i:] { - if net.GetConn(a, b) != nil { - cc++ - } - } - } - - if cc != 1 { - t.Errorf("expected one connection, got %v", cc) - } -} - -func TestConnectNodesFull(t *testing.T) { - 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: 2}, - {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) - }) - } -} - -func verifyFull(t *testing.T, net *Network, ids []enode.ID) { - t.Helper() - n := len(ids) - var connections int - for i, lid := range ids { - for _, rid := range ids[i+1:] { - if net.GetConn(lid, rid) != nil { - connections++ - } - } - } - - want := n * (n - 1) / 2 - if connections != want { - t.Errorf("wrong number of connections, got: %v, want: %v", connections, want) - } -} - func TestConnectNodesChain(t *testing.T) { net, ids := newTestNetwork(t, 10) defer net.Shutdown() @@ -211,36 +118,3 @@ func verifyRing(t *testing.T, net *Network, ids []enode.ID) { } } } - -func TestConnectNodesStar(t *testing.T) { - net, ids := newTestNetwork(t, 10) - defer net.Shutdown() - - pivotIndex := 2 - - err := net.ConnectNodesStar(ids, ids[pivotIndex]) - if err != nil { - t.Fatal(err) - } - - verifyStar(t, net, ids, pivotIndex) -} - -func verifyStar(t *testing.T, net *Network, ids []enode.ID, centerIndex int) { - t.Helper() - n := len(ids) - for i := 0; i < n; i++ { - for j := i + 1; j < n; j++ { - c := net.GetConn(ids[i], ids[j]) - if i == centerIndex || j == centerIndex { - if c == nil { - t.Errorf("nodes %v and %v are not connected, but they should be", i, j) - } - } else { - if c != nil { - t.Errorf("nodes %v and %v are connected, but they should not be", i, j) - } - } - } - } -} From b56ecc290936db277b9a21911fa98ebaae52c150 Mon Sep 17 00:00:00 2001 From: Ferenc Szabo Date: Wed, 26 Dec 2018 19:08:32 +0100 Subject: [PATCH 16/18] p2p/simulations: fix code inspection problems - typos - unused global variable - unused function parameters - redundant import alias - code style issue: snake case --- p2p/simulations/adapters/exec.go | 2 +- p2p/simulations/adapters/inproc.go | 2 +- p2p/simulations/connect.go | 5 ----- p2p/simulations/connect_test.go | 1 + p2p/simulations/http_test.go | 3 ++- p2p/simulations/mocker_test.go | 14 +++++++------- p2p/simulations/network.go | 6 +++--- 7 files changed, 15 insertions(+), 18 deletions(-) diff --git a/p2p/simulations/adapters/exec.go b/p2p/simulations/adapters/exec.go index abb1967171..9b588db1be 100644 --- a/p2p/simulations/adapters/exec.go +++ b/p2p/simulations/adapters/exec.go @@ -46,7 +46,7 @@ import ( func init() { // Register a reexec function to start a simulation node when the current binary is - // executed as "p2p-node" (rather than whataver the main() function would normally do). + // executed as "p2p-node" (rather than whatever the main() function would normally do). reexec.Register("p2p-node", execP2PNode) } diff --git a/p2p/simulations/adapters/inproc.go b/p2p/simulations/adapters/inproc.go index 0ebc66b2d7..eada9579ed 100644 --- a/p2p/simulations/adapters/inproc.go +++ b/p2p/simulations/adapters/inproc.go @@ -130,7 +130,7 @@ func (s *SimAdapter) Dial(dest *enode.Node) (conn net.Conn, err error) { return nil, err } // this is simulated 'listening' - // asynchronously call the dialed destintion node's p2p server + // asynchronously call the dialed destination node's p2p server // to set up connection on the 'listening' side go srv.SetupConn(pipe1, 0, nil) return pipe2, nil diff --git a/p2p/simulations/connect.go b/p2p/simulations/connect.go index 094de579ff..b15f14b089 100644 --- a/p2p/simulations/connect.go +++ b/p2p/simulations/connect.go @@ -17,16 +17,11 @@ package simulations import ( - "errors" "strings" "github.com/ethereum/go-ethereum/p2p/enode" ) -var ( - ErrNodeNotFound = errors.New("node not found") -) - // 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) { diff --git a/p2p/simulations/connect_test.go b/p2p/simulations/connect_test.go index 88b58b8fa0..0e30e88d2b 100644 --- a/p2p/simulations/connect_test.go +++ b/p2p/simulations/connect_test.go @@ -25,6 +25,7 @@ import ( ) func newTestNetwork(t *testing.T, nodeCount int) (*Network, []enode.ID) { + t.Helper() adapter := adapters.NewSimAdapter(adapters.Services{ "noopwoop": func(ctx *adapters.ServiceContext) (node.Service, error) { return NewNoopService(nil), nil diff --git a/p2p/simulations/http_test.go b/p2p/simulations/http_test.go index d9513caaa0..c0a5acb3d8 100644 --- a/p2p/simulations/http_test.go +++ b/p2p/simulations/http_test.go @@ -35,7 +35,7 @@ import ( "github.com/ethereum/go-ethereum/p2p/enode" "github.com/ethereum/go-ethereum/p2p/simulations/adapters" "github.com/ethereum/go-ethereum/rpc" - colorable "github.com/mattn/go-colorable" + "github.com/mattn/go-colorable" ) var ( @@ -294,6 +294,7 @@ var testServices = adapters.Services{ } func testHTTPServer(t *testing.T) (*Network, *httptest.Server) { + t.Helper() adapter := adapters.NewSimAdapter(testServices) network := NewNetwork(adapter, &NetworkConfig{ DefaultService: "test", diff --git a/p2p/simulations/mocker_test.go b/p2p/simulations/mocker_test.go index 7c7016a5e6..192be1732c 100644 --- a/p2p/simulations/mocker_test.go +++ b/p2p/simulations/mocker_test.go @@ -15,7 +15,7 @@ // along with the go-ethereum library. If not, see . // Package simulations simulates p2p networks. -// A mokcer simulates starting and stopping real nodes in a network. +// A mocker simulates starting and stopping real nodes in a network. package simulations import ( @@ -135,13 +135,13 @@ func TestMocker(t *testing.T) { wg.Wait() //check there are nodeCount number of nodes in the network - nodes_info, err := client.GetNodes() + nodesInfo, err := client.GetNodes() if err != nil { t.Fatalf("Could not get nodes list: %s", err) } - if len(nodes_info) != nodeCount { - t.Fatalf("Expected %d number of nodes, got: %d", nodeCount, len(nodes_info)) + if len(nodesInfo) != nodeCount { + t.Fatalf("Expected %d number of nodes, got: %d", nodeCount, len(nodesInfo)) } //stop the mocker @@ -160,12 +160,12 @@ func TestMocker(t *testing.T) { } //now the number of nodes in the network should be zero - nodes_info, err = client.GetNodes() + nodesInfo, err = client.GetNodes() if err != nil { t.Fatalf("Could not get nodes list: %s", err) } - if len(nodes_info) != 0 { - t.Fatalf("Expected empty list of nodes, got: %d", len(nodes_info)) + if len(nodesInfo) != 0 { + t.Fatalf("Expected empty list of nodes, got: %d", len(nodesInfo)) } } diff --git a/p2p/simulations/network.go b/p2p/simulations/network.go index 237ad9e4d3..86f7dc9bef 100644 --- a/p2p/simulations/network.go +++ b/p2p/simulations/network.go @@ -516,7 +516,7 @@ func (net *Network) getConn(oneID, otherID enode.ID) *Conn { return net.Conns[i] } -// InitConn(one, other) retrieves the connectiton model for the connection between +// InitConn(one, other) retrieves the connection model for the connection between // peers one and other, or creates a new one if it does not exist // the order of nodes does not matter, i.e., Conn(i,j) == Conn(j, i) // it checks if the connection is already up, and if the nodes are running @@ -562,8 +562,8 @@ func (net *Network) Shutdown() { close(net.quitc) } -//Reset resets all network properties: -//emtpies the nodes and the connection list +// Reset resets all network properties: +// empties the nodes and the connection list func (net *Network) Reset() { net.lock.Lock() defer net.lock.Unlock() From 75eeaa0743e72c79275d27640737ba0cea556142 Mon Sep 17 00:00:00 2001 From: Ferenc Szabo Date: Wed, 26 Dec 2018 19:10:12 +0100 Subject: [PATCH 17/18] swarm/network: fix code inspection problems - typos - redundant import alias --- swarm/network/simulation/bucket.go | 2 +- swarm/network/simulation/bucket_test.go | 2 +- swarm/network/simulation/example_test.go | 2 +- swarm/network/simulation/kademlia.go | 2 +- swarm/network/simulation/node.go | 6 +++--- swarm/network/simulation/simulation_test.go | 2 +- swarm/network_test.go | 4 ++-- 7 files changed, 10 insertions(+), 10 deletions(-) diff --git a/swarm/network/simulation/bucket.go b/swarm/network/simulation/bucket.go index bd15ea2ab1..49a1f43091 100644 --- a/swarm/network/simulation/bucket.go +++ b/swarm/network/simulation/bucket.go @@ -21,7 +21,7 @@ import "github.com/ethereum/go-ethereum/p2p/enode" // BucketKey is the type that should be used for keys in simulation buckets. type BucketKey string -// NodeItem returns an item set in ServiceFunc function for a particualar node. +// NodeItem returns an item set in ServiceFunc function for a particular node. func (s *Simulation) NodeItem(id enode.ID, key interface{}) (value interface{}, ok bool) { s.mu.Lock() defer s.mu.Unlock() diff --git a/swarm/network/simulation/bucket_test.go b/swarm/network/simulation/bucket_test.go index dbe3e93a6a..e649e88164 100644 --- a/swarm/network/simulation/bucket_test.go +++ b/swarm/network/simulation/bucket_test.go @@ -24,7 +24,7 @@ import ( "github.com/ethereum/go-ethereum/p2p/simulations/adapters" ) -// TestServiceBucket tests all bucket functionalities using subtests. +// TestServiceBucket tests all bucket functionality using subtests. // It constructs a simulation of two nodes by adding items to their buckets // in ServiceFunc constructor, then by SetNodeItem. Testing UpNodesItems // is done by stopping one node and validating availability of its items. diff --git a/swarm/network/simulation/example_test.go b/swarm/network/simulation/example_test.go index a100ede516..e9a360dfd4 100644 --- a/swarm/network/simulation/example_test.go +++ b/swarm/network/simulation/example_test.go @@ -25,7 +25,7 @@ import ( // Every node can have a Kademlia associated using the node bucket under // BucketKeyKademlia key. This allows to use WaitTillHealthy to block until -// all nodes have the their Kadmlias healthy. +// all nodes have the their Kademlias healthy. func ExampleSimulation_WaitTillHealthy() { log.Error("temporarily disabled as simulations.WaitTillHealthy cannot be trusted") diff --git a/swarm/network/simulation/kademlia.go b/swarm/network/simulation/kademlia.go index 25bb0f6a9f..ebec468f14 100644 --- a/swarm/network/simulation/kademlia.go +++ b/swarm/network/simulation/kademlia.go @@ -28,7 +28,7 @@ import ( ) // BucketKeyKademlia is the key to be used for storing the kademlia -// instance for particuar node, usually inside the ServiceFunc function. +// instance for particular node, usually inside the ServiceFunc function. var BucketKeyKademlia BucketKey = "kademlia" // WaitTillHealthy is blocking until the health of all kademlias is true. diff --git a/swarm/network/simulation/node.go b/swarm/network/simulation/node.go index 110455c1ad..c78f4d4e28 100644 --- a/swarm/network/simulation/node.go +++ b/swarm/network/simulation/node.go @@ -89,9 +89,9 @@ func (s *Simulation) AddNodes(count int) (ids []enode.ID, err error) { return ids, nil } -//UploadSnapshot uploads a snapshot to the simulation -//This method tries to open the json file provided, applies the config to all nodes -//and then loads the snapshot into the Simulation network +// UploadSnapshot uploads a snapshot to the simulation +// This method tries to open the json file provided, applies the config to +// all nodes and then loads the snapshot into the Simulation network func (s *Simulation) UploadSnapshot(snapshotFile string) error { f, err := os.Open(snapshotFile) if err != nil { diff --git a/swarm/network/simulation/simulation_test.go b/swarm/network/simulation/simulation_test.go index abbe4826ef..f837f93824 100644 --- a/swarm/network/simulation/simulation_test.go +++ b/swarm/network/simulation/simulation_test.go @@ -28,7 +28,7 @@ import ( "github.com/ethereum/go-ethereum/node" "github.com/ethereum/go-ethereum/p2p/simulations" "github.com/ethereum/go-ethereum/p2p/simulations/adapters" - colorable "github.com/mattn/go-colorable" + "github.com/mattn/go-colorable" ) var ( diff --git a/swarm/network_test.go b/swarm/network_test.go index 2e093908dd..54b34a77ff 100644 --- a/swarm/network_test.go +++ b/swarm/network_test.go @@ -316,7 +316,7 @@ func testSwarmNetwork(t *testing.T, o *testSwarmNetworkOptions, steps ...testSwa change := step.nodeCount - len(sim.UpNodeIDs()) if change > 0 { - addNodesAndPreserveChainTopology(t, sim, change) + addNodesChain(t, sim, change) } else if change < 0 { _, err := sim.StopRandomNodes(-change) if err != nil { @@ -371,7 +371,7 @@ func testSwarmNetwork(t *testing.T, o *testSwarmNetworkOptions, steps ...testSwa } } -func addNodesAndPreserveChainTopology(t *testing.T, s *simulation.Simulation, increment int) { +func addNodesChain(t *testing.T, s *simulation.Simulation, increment int) { t.Helper() if increment < 1 { From 93c5c3f796fea5351b969cd1cffd4ce6d7ce080b Mon Sep 17 00:00:00 2001 From: Ferenc Szabo Date: Fri, 28 Dec 2018 13:52:00 +0100 Subject: [PATCH 18/18] swarm/network: bring back AddNodeOption for AddNode() During my code cleanup efforts I removed the AddNodeOption type and related functionality. As that was not used anywhere. However, during code review I was asked to bring back the functionality. Here I'm doing that; still, I would prefer to follow YAGNI. --- swarm/network/simulation/node.go | 29 ++++++++++++++++++++++----- swarm/network/simulation/node_test.go | 23 +++++++++++++++++++++ 2 files changed, 47 insertions(+), 5 deletions(-) diff --git a/swarm/network/simulation/node.go b/swarm/network/simulation/node.go index c78f4d4e28..5d70a1556c 100644 --- a/swarm/network/simulation/node.go +++ b/swarm/network/simulation/node.go @@ -61,10 +61,26 @@ func (s *Simulation) DownNodeIDs() (ids []enode.ID) { return ids } +// AddNodeOption defines the option that can be passed +// to Simulation.AddNode method. +type AddNodeOption func(*adapters.NodeConfig) + +// AddNodeWithMsgEvents sets the EnableMsgEvents option +// to NodeConfig. +func AddNodeWithMsgEvents(enable bool) AddNodeOption { + return func(o *adapters.NodeConfig) { + o.EnableMsgEvents = enable + } +} + // AddNode creates a new node with random configuration and adds that to -// the network. All services will be started on the node. -func (s *Simulation) AddNode() (id enode.ID, err error) { +// the network. All services will be started on the node. Tweaking the node's +// configuration is possible through the opts parameter. +func (s *Simulation) AddNode(opts ...AddNodeOption) (id enode.ID, err error) { conf := adapters.RandomNodeConfig() + for _, o := range opts { + o(conf) + } if len(conf.Services) == 0 { conf.Services = s.serviceNames } @@ -77,10 +93,10 @@ func (s *Simulation) AddNode() (id enode.ID, err error) { // AddNodes creates new nodes with random configurations, // applies provided options to the config and adds nodes to network. -func (s *Simulation) AddNodes(count int) (ids []enode.ID, err error) { +func (s *Simulation) AddNodes(count int, opts ...AddNodeOption) (ids []enode.ID, err error) { ids = make([]enode.ID, 0, count) for i := 0; i < count; i++ { - id, err := s.AddNode() + id, err := s.AddNode(opts...) if err != nil { return nil, err } @@ -92,7 +108,7 @@ func (s *Simulation) AddNodes(count int) (ids []enode.ID, err error) { // UploadSnapshot uploads a snapshot to the simulation // This method tries to open the json file provided, applies the config to // all nodes and then loads the snapshot into the Simulation network -func (s *Simulation) UploadSnapshot(snapshotFile string) error { +func (s *Simulation) UploadSnapshot(snapshotFile string, opts ...AddNodeOption) error { f, err := os.Open(snapshotFile) if err != nil { return err @@ -119,6 +135,9 @@ func (s *Simulation) UploadSnapshot(snapshotFile string) error { for _, n := range snap.Nodes { n.Node.Config.EnableMsgEvents = true n.Node.Config.Services = s.serviceNames + for _, o := range opts { + o(n.Node.Config) + } } log.Info("Waiting for p2p connections to be established...") diff --git a/swarm/network/simulation/node_test.go b/swarm/network/simulation/node_test.go index eb4cf48569..4d682ab547 100644 --- a/swarm/network/simulation/node_test.go +++ b/swarm/network/simulation/node_test.go @@ -30,6 +30,29 @@ import ( "github.com/ethereum/go-ethereum/swarm/network" ) +func TestAddNodeWithMsgEvents(t *testing.T) { + sim := New(noopServiceFuncMap) + defer sim.Close() + + id, err := sim.AddNode(AddNodeWithMsgEvents(true)) + if err != nil { + t.Fatal(err) + } + + if !sim.Net.GetNode(id).Config.EnableMsgEvents { + t.Error("EnableMsgEvents is false") + } + + id, err = sim.AddNode(AddNodeWithMsgEvents(false)) + if err != nil { + t.Fatal(err) + } + + if sim.Net.GetNode(id).Config.EnableMsgEvents { + t.Error("EnableMsgEvents is true") + } +} + func TestUpDownNodeIDs(t *testing.T) { sim := New(noopServiceFuncMap) defer sim.Close()