address PR comments

cmd/swarm/swarm-snapshot/create.go

@@ -39,7 +39,7 @@ import (
 )
 
 const testMinProxBinSize = 2
-const NoConnectionTimeout = 1
+const NoConnectionTimeout = 2 * time.Second
 
 func create(ctx *cli.Context) error {
 	log.PrintOrigins(true)
@@ -61,14 +61,22 @@ func create(ctx *cli.Context) error {
 }
 
 func discoverySnapshot(filename string, nodes int) error {
-	log.Debug("discoverySnapshot", "filename", filename, "nodes", nodes)
 	//disable discovery if topology is specified
 	discovery = topology == ""
-	ids := make([]enode.ID, 0)
+	log.Debug("discoverySnapshot", "filename", filename, "nodes", nodes, "discovery", discovery)
+	i := 0
+	var lock sync.Mutex
+	var pivotNodeID enode.ID
 	sim := simulation.New(map[string]simulation.ServiceFunc{
 		"bzz": func(ctx *adapters.ServiceContext, b *sync.Map) (node.Service, func(), error) {
-			addr := network.NewAddr(ctx.Config.Node())
+			lock.Lock()
+			i++
+			if i == pivot {
+				pivotNodeID = ctx.Config.ID
+			}
+			lock.Unlock()
 
+			addr := network.NewAddr(ctx.Config.Node())
 			kp := network.NewKadParams()
 			kp.MinProxBinSize = testMinProxBinSize
 
@@ -77,16 +85,12 @@ func discoverySnapshot(filename string, nodes int) error {
 			hp.KeepAliveInterval = time.Duration(200) * time.Millisecond
 			hp.Discovery = discovery
 
-			log.Info(fmt.Sprintf("discovery for nodeID %s is %t", ctx.Config.ID.String(), hp.Discovery))
-
 			config := &network.BzzConfig{
 				OverlayAddr:  addr.Over(),
 				UnderlayAddr: addr.Under(),
 				HiveParams:   hp,
 			}
-			ids = append(ids, ctx.Config.ID)
 			return network.NewBzz(config, kad, nil, nil, nil), nil, nil
-
 		},
 	})
 	defer sim.Close()
@@ -104,7 +108,7 @@ func discoverySnapshot(filename string, nodes int) error {
 		if ev.Type == simulations.EventTypeConn {
 			utils.Fatalf("this shouldn't happen as connections weren't initiated yet")
 		}
-	case <-time.After(NoConnectionTimeout * time.Second):
+	case <-time.After(NoConnectionTimeout):
 	}
 
 	sub.Unsubscribe()
@@ -115,27 +119,21 @@ func discoverySnapshot(filename string, nodes int) error {
 
 	switch topology {
 	case "star":
-		sim.Net.SetPivotNode(ids[pivot])
-		if err := sim.Net.ConnectNodesStarPivot(nil); err != nil {
-			utils.Fatalf("had an error connecting the nodes in a star: %v", err)
-		}
+		sim.Net.SetPivotNode(pivotNodeID)
+		err = sim.Net.ConnectNodesStarPivot(nil)
 	case "ring":
-		if err := sim.Net.ConnectNodesRing(nil); err != nil {
-			utils.Fatalf("had an error connecting the nodes in a ring: %v", err)
-		}
+		err = sim.Net.ConnectNodesRing(nil)
 	case "chain":
-		if err := sim.Net.ConnectNodesChain(nil); err != nil {
-			utils.Fatalf("had an error connecting the nodes in a chain: %v", err)
-		}
+		err = sim.Net.ConnectNodesChain(nil)
 	case "full":
-		if err := sim.Net.ConnectNodesFull(nil); err != nil {
-			utils.Fatalf("had an error connecting full: %v", err)
-		}
+		err = sim.Net.ConnectNodesFull(nil)
 	default:
 		// no topology specified = connect ring and await discovery
-		if err := sim.Net.ConnectNodesRing(nil); err != nil {
-			utils.Fatalf("had an error connecting ring: %v", err)
-		}
+		topology = "ring"
+		err = sim.Net.ConnectNodesRing(nil)
+	}
+	if err != nil {
+		utils.Fatalf("had an error connecting the nodes in a %v topology: %v", topology, err)
 	}
 
 	if discovery {
@@ -172,7 +170,7 @@ func discoverySnapshot(filename string, nodes int) error {
 	if err != nil {
 		return fmt.Errorf("corrupt json snapshot: %v", err)
 	}
-	err = ioutil.WriteFile(filename, jsonsnapshot, 0755)
+	err = ioutil.WriteFile(filename, jsonsnapshot, 0666)
 	if err != nil {
 		return err
 	}

cmd/swarm/swarm-snapshot/main.go

@@ -40,10 +40,6 @@ var (
 var app = utils.NewApp("", "Swarm Snapshot Util")
 var discovery = true
 
-func init() {
-	//adapters.RegisterServices(serviceFuncs)
-}
-
 func init() {
 	log.PrintOrigins(true)
 	log.Root().SetHandler(log.LvlFilterHandler(log.Lvl(verbosity), log.StreamHandler(os.Stdout, log.TerminalFormat(true))))
@@ -101,10 +97,6 @@ func init() {
 
 	sort.Sort(cli.FlagsByName(app.Flags))
 	sort.Sort(cli.CommandsByName(app.Commands))
-	app.Before = func(ctx *cli.Context) error {
-
-		return nil
-	}
 }
 
 func main() {
@@ -116,30 +108,3 @@ func main() {
 	}
 	os.Exit(0)
 }
-
-/*
-var serviceFuncs = adapters.Services{
-	"discovery": newService,
-}
-
-func newService(ctx *adapters.ServiceContext) (node.Service, error) {
-	addr := network.NewAddr(ctx.Config.Node())
-
-	kp := network.NewKadParams()
-	kp.MinProxBinSize = testMinProxBinSize
-
-	kad := network.NewKademlia(addr.Over(), kp)
-	hp := network.NewHiveParams()
-	hp.KeepAliveInterval = time.Duration(200) * time.Millisecond
-	hp.Discovery = discovery
-
-	log.Info(fmt.Sprintf("discovery for nodeID %s is %t", ctx.Config.ID.String(), hp.Discovery))
-
-	config := &network.BzzConfig{
-		OverlayAddr:  addr.Over(),
-		UnderlayAddr: addr.Under(),
-		HiveParams:   hp,
-	}
-
-	return network.NewBzz(config, kad, nil, nil, nil), nil
-}*/

cmd/swarm/swarm-snapshot/verify.go

@@ -18,19 +18,12 @@ package main
 
 import (
 	"context"
-	"encoding/json"
 	"errors"
-	"fmt"
-	"io/ioutil"
-	"os"
 	"sync"
 	"time"
 
 	"github.com/ethereum/go-ethereum/cmd/utils"
-	"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"
 	"github.com/ethereum/go-ethereum/swarm/network/simulation"
@@ -51,36 +44,9 @@ func verify(ctx *cli.Context) error {
 	}
 
 	return err
-
 }
 
 func verifySnapshot(filename string) error {
-	f, err := os.Open(filename)
-	if err != nil {
-		return err
-	}
-	defer func() {
-		err := f.Close()
-		if err != nil {
-			log.Error("Error closing snapshot file", "err", err)
-		}
-	}()
-	jsonbyte, err := ioutil.ReadAll(f)
-	if err != nil {
-		return err
-	}
-	var snap simulations.Snapshot
-	err = json.Unmarshal(jsonbyte, &snap)
-	if err != nil {
-		return err
-	}
-
-	for _, n := range snap.Nodes {
-		n.Node.Config.EnableMsgEvents = true
-	}
-
-	//	discovery := true
-	ids := make([]enode.ID, 0)
 	sim := simulation.New(map[string]simulation.ServiceFunc{
 		"bzz": func(ctx *adapters.ServiceContext, b *sync.Map) (node.Service, func(), error) {
 			addr := network.NewAddr(ctx.Config.Node())
@@ -93,20 +59,17 @@ func verifySnapshot(filename string) error {
 			hp.KeepAliveInterval = time.Duration(200) * time.Millisecond
 			hp.Discovery = true //discovery
 
-			log.Info(fmt.Sprintf("discovery for nodeID %s is %t", ctx.Config.ID.String(), hp.Discovery))
-
 			config := &network.BzzConfig{
 				OverlayAddr:  addr.Over(),
 				UnderlayAddr: addr.Under(),
 				HiveParams:   hp,
 			}
-			ids = append(ids, ctx.Config.ID)
 			return network.NewBzz(config, kad, nil, nil, nil), nil, nil
 
 		},
 	})
 	defer sim.Close()
-	err = sim.UploadSnapshot("snapshot.json")
+	err := sim.UploadSnapshot(filename)
 	if err != nil {
 		utils.Fatalf("%v", err)
 	}

cmd/swarm/swarm-snapshot/verify_test.go

@@ -23,7 +23,7 @@ import (
 func TestSnapshotVerify(t *testing.T) {
 	snap := runSnapshot(t,
 		"v",
-		"snapshot.json",
+		"testdata/snapshot.json",
 	)
 
 	snap.ExpectExit()

internal/cmdtest/test_cmd.go

@@ -227,7 +227,7 @@ func (tt *TestCmd) Kill() {
 }
 
 func (tt *TestCmd) withKillTimeout(fn func()) {
-	timeout := time.AfterFunc(15*time.Second, func() {
+	timeout := time.AfterFunc(5*time.Second, func() {
 		tt.Log("killing the child process (timeout)")
 		tt.Kill()
 	})

swarm/network/simulation/kademlia.go

@@ -94,53 +94,3 @@ func (s *Simulation) kademlias() (ks map[enode.ID]*network.Kademlia) {
 	}
 	return ks
 }
-
-/*func (s *Simulation) WaitNetworkHealth() error {
-	// construct the peer pot, so that kademlia health can be checked
-	addrs := make([][]byte, len(s.Net.Nodes))
-	ids := make([]enode.ID, len(s.Net.Nodes))
-	for i, v := range s.Net.Nodes {
-		ids[i] = v.ID()
-		addrs[i] = v.ID().Bytes()
-	}
-	// construct the peer pot, so that kademlia health can be checked
-	ppmap := network.NewPeerPotMap(2, addrs) //yes i'm hardcoding this
-	check := func(ctx context.Context, id enode.ID) (bool, error) {
-		select {
-		case <-ctx.Done():
-			return false, ctx.Err()
-		default:
-		}
-
-		node := s.Net.GetNode(id)
-		if node == nil {
-			return false, fmt.Errorf("unknown node: %s", id)
-		}
-		client, err := node.Client()
-		if err != nil {
-			return false, fmt.Errorf("error getting node client: %s", err)
-		}
-		healthy := &network.Health{}
-		if err := client.Call(&healthy, "hive_healthy", ppmap[id.String()]); err != nil {
-			return false, fmt.Errorf("error getting node health: %s", err)
-		}
-		log.Debug(fmt.Sprintf("node %4s healthy: got nearest neighbours: %v, know nearest neighbours: %v, saturated: %v\n%v", id, healthy.GotNN, healthy.KnowNN, healthy.Full))
-		return healthy.KnowNN && healthy.GotNN && healthy.Full, nil
-	}
-
-	// 64 nodes ~ 1min
-	// 128 nodes ~
-	timeout := 30 * time.Second
-	ctx, cancel := context.WithTimeout(context.Background(), timeout)
-	defer cancel()
-	trigger := make(chan enode.ID)
-
-	result := simulations.NewSimulation(s.Net).Run(ctx, &simulations.Step{
-		Trigger: trigger,
-		Expect: &simulations.Expectation{
-			Nodes: ids,
-			Check: check,
-		},
-	})
-	return result.Error
-}*/