p2p/simulations: fix lock copying (found by go vet)

p2p/simulations/events.go

@@ -73,8 +73,7 @@ func NewEvent(v interface{}) *Event {
 	switch v := v.(type) {
 	case *Node:
 		event.Type = EventTypeNode
-		node := *v
-		event.Node = &node
+		event.Node = v.copy()
 	case *Conn:
 		event.Type = EventTypeConn
 		conn := *v

p2p/simulations/http_test.go

@@ -420,16 +420,8 @@ type expectEvents struct {
 }
 
 func (t *expectEvents) nodeEvent(id string, up bool) *Event {
-	node := Node{
-		Config: &adapters.NodeConfig{
-			ID: enode.HexID(id),
-		},
-		up: up,
-	}
-	return &Event{
-		Type: EventTypeNode,
-		Node: &node,
-	}
+	config := &adapters.NodeConfig{ID: enode.HexID(id)}
+	return &Event{Type: EventTypeNode, Node: newNode(nil, config, up)}
 }
 
 func (t *expectEvents) connEvent(one, other string, up bool) *Event {
@@ -450,7 +442,7 @@ loop:
 	for {
 		select {
 		case event := <-t.events:
-			t.Logf("received %s event: %s", event.Type, event)
+			t.Logf("received %s event: %v", event.Type, event)
 
 			if event.Type != EventTypeMsg || event.Msg.Received {
 				continue loop
@@ -486,7 +478,7 @@ func (t *expectEvents) expect(events ...*Event) {
 	for {
 		select {
 		case event := <-t.events:
-			t.Logf("received %s event: %s", event.Type, event)
+			t.Logf("received %s event: %v", event.Type, event)
 
 			expected := events[i]
 			if event.Type != expected.Type {

p2p/simulations/network.go

@@ -119,10 +119,7 @@ func (net *Network) NewNodeWithConfig(conf *adapters.NodeConfig) (*Node, error)
 	if err != nil {
 		return nil, err
 	}
-	node := &Node{
-		Node:   adapterNode,
-		Config: conf,
-	}
+	node := newNode(adapterNode, conf, false)
 	log.Trace("Node created", "id", conf.ID)
 
 	nodeIndex := len(net.Nodes)
@@ -448,7 +445,7 @@ func (net *Network) GetNodeIDs(excludeIDs ...enode.ID) []enode.ID {
 }
 
 func (net *Network) getNodeIDs(excludeIDs []enode.ID) []enode.ID {
-	// Get all curent nodeIDs
+	// Get all current nodeIDs
 	nodeIDs := make([]enode.ID, 0, len(net.nodeMap))
 	for id := range net.nodeMap {
 		nodeIDs = append(nodeIDs, id)
@@ -735,7 +732,16 @@ type Node struct {
 
 	// up tracks whether or not the node is running
 	up   bool
-	upMu sync.RWMutex
+	upMu *sync.RWMutex
+}
+
+func newNode(an adapters.Node, ac *adapters.NodeConfig, up bool) *Node {
+	return &Node{Node: an, Config: ac, up: up, upMu: new(sync.RWMutex)}
+}
+
+func (n *Node) copy() *Node {
+	configCpy := *n.Config
+	return newNode(n.Node, &configCpy, n.Up())
 }
 
 // Up returns whether the node is currently up (online)
@@ -787,22 +793,19 @@ func (n *Node) MarshalJSON() ([]byte, error) {
 	})
 }
 
-// UnmarshalJSON implements json.Unmarshaler interface so that we don't lose
-// Node.up status. IMPORTANT: The implementation is incomplete; we lose
-// p2p.NodeInfo.
+// UnmarshalJSON implements json.Unmarshaler interface so that we don't lose Node.up
+// status. IMPORTANT: The implementation is incomplete; we lose p2p.NodeInfo.
 func (n *Node) UnmarshalJSON(raw []byte) error {
 	// TODO: How should we turn back NodeInfo into n.Node?
 	// Ticket: https://github.com/ethersphere/go-ethereum/issues/1177
-	node := struct {
+	var node struct {
 		Config *adapters.NodeConfig `json:"config,omitempty"`
 		Up     bool                 `json:"up"`
-	}{}
+	}
 	if err := json.Unmarshal(raw, &node); err != nil {
 		return err
 	}
-
-	n.SetUp(node.Up)
-	n.Config = node.Config
+	*n = *newNode(nil, node.Config, node.Up)
 	return nil
 }
 
@@ -899,7 +902,7 @@ func (net *Network) snapshot(addServices []string, removeServices []string) (*Sn
 		Nodes: make([]NodeSnapshot, len(net.Nodes)),
 	}
 	for i, node := range net.Nodes {
-		snap.Nodes[i] = NodeSnapshot{Node: *node}
+		snap.Nodes[i] = NodeSnapshot{Node: *node.copy()}
 		if !node.Up() {
 			continue
 		}

p2p/simulations/network_test.go

@@ -758,27 +758,22 @@ func benchmarkMinimalServiceTmp(b *testing.B) {
 }
 
 func TestNode_UnmarshalJSON(t *testing.T) {
-	t.Run(
-		"test unmarshal of Node up field",
-		func(t *testing.T) {
-			runNodeUnmarshalJSON(t, casesNodeUnmarshalJSONUpField())
-		},
-	)
-	t.Run(
-		"test unmarshal of Node Config field",
-		func(t *testing.T) {
-			runNodeUnmarshalJSON(t, casesNodeUnmarshalJSONConfigField())
-		},
-	)
+	t.Run("up_field", func(t *testing.T) {
+		runNodeUnmarshalJSON(t, casesNodeUnmarshalJSONUpField())
+	})
+	t.Run("config_field", func(t *testing.T) {
+		runNodeUnmarshalJSON(t, casesNodeUnmarshalJSONConfigField())
+	})
 }
 
 func runNodeUnmarshalJSON(t *testing.T, tests []nodeUnmarshalTestCase) {
 	t.Helper()
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			var got Node
-			if err := got.UnmarshalJSON([]byte(tt.marshaled)); err != nil {
+			var got *Node
+			if err := json.Unmarshal([]byte(tt.marshaled), &got); err != nil {
 				expectErrorMessageToContain(t, err, tt.wantErr)
+				got = nil
 			}
 			expectNodeEquality(t, got, tt.want)
 		})
@@ -788,7 +783,7 @@ func runNodeUnmarshalJSON(t *testing.T, tests []nodeUnmarshalTestCase) {
 type nodeUnmarshalTestCase struct {
 	name      string
 	marshaled string
-	want      Node
+	want      *Node
 	wantErr   string
 }
 
@@ -812,7 +807,7 @@ func expectErrorMessageToContain(t *testing.T, got error, want string) {
 	}
 }
 
-func expectNodeEquality(t *testing.T, got Node, want Node) {
+func expectNodeEquality(t *testing.T, got, want *Node) {
 	t.Helper()
 	if !reflect.DeepEqual(got, want) {
 		t.Errorf("Node.UnmarshalJSON() = %v, want %v", got, want)
@@ -824,23 +819,17 @@ func casesNodeUnmarshalJSONUpField() []nodeUnmarshalTestCase {
 		{
 			name:      "empty json",
 			marshaled: "{}",
-			want: Node{
-				up: false,
-			},
+			want:      newNode(nil, nil, false),
 		},
 		{
 			name:      "a stopped node",
 			marshaled: "{\"up\": false}",
-			want: Node{
-				up: false,
-			},
+			want:      newNode(nil, nil, false),
 		},
 		{
 			name:      "a running node",
 			marshaled: "{\"up\": true}",
-			want: Node{
-				up: true,
-			},
+			want:      newNode(nil, nil, true),
 		},
 		{
 			name:      "invalid JSON value on valid key",
@@ -867,26 +856,17 @@ func casesNodeUnmarshalJSONConfigField() []nodeUnmarshalTestCase {
 		{
 			name:      "Config field is omitted",
 			marshaled: "{}",
-			want: Node{
-				Config: nil,
-			},
+			want:      newNode(nil, nil, false),
 		},
 		{
 			name:      "Config field is nil",
-			marshaled: "{\"config\": nil}",
-			want: Node{
-				Config: nil,
-			},
+			marshaled: "{\"config\": null}",
+			want:      newNode(nil, nil, false),
 		},
 		{
 			name:      "a non default Config field",
 			marshaled: "{\"config\":{\"name\":\"node_ecdd0\",\"port\":44665}}",
-			want: Node{
-				Config: &adapters.NodeConfig{
-					Name: "node_ecdd0",
-					Port: 44665,
-				},
-			},
+			want:      newNode(nil, &adapters.NodeConfig{Name: "node_ecdd0", Port: 44665}, false),
 		},
 	}
 }