cmd, eth, p2p: fix review issues enumerated by Felix

Author: karalabe

cmd/geth/admin.go

@@ -25,7 +25,7 @@ func (js *jsre) adminBindings() {
 	js.re.Set("admin", struct{}{})
 	t, _ := js.re.Get("admin")
 	admin := t.Object()
-	admin.Set("trustPeer", js.trustPeer)
+	admin.Set("addPeer", js.addPeer)
 	admin.Set("startRPC", js.startRPC)
 	admin.Set("stopRPC", js.stopRPC)
 	admin.Set("nodeInfo", js.nodeInfo)
@@ -243,13 +243,13 @@ func (js *jsre) stopRPC(call otto.FunctionCall) otto.Value {
 	return otto.FalseValue()
 }
 
-func (js *jsre) trustPeer(call otto.FunctionCall) otto.Value {
+func (js *jsre) addPeer(call otto.FunctionCall) otto.Value {
 	nodeURL, err := call.Argument(0).ToString()
 	if err != nil {
 		fmt.Println(err)
 		return otto.FalseValue()
 	}
-	err = js.ethereum.TrustPeer(nodeURL)
+	err = js.ethereum.AddPeer(nodeURL)
 	if err != nil {
 		fmt.Println(err)
 		return otto.FalseValue()

cmd/geth/main.go

@@ -232,7 +232,7 @@ JavaScript API. See https://github.com/ethereum/go-ethereum/wiki/Javascipt-Conso
 		utils.IdentityFlag,
 		utils.UnlockedAccountFlag,
 		utils.PasswordFileFlag,
-		utils.BootNodesFlag,
+		utils.BootnodesFlag,
 		utils.DataDirFlag,
 		utils.BlockchainVersionFlag,
 		utils.JSpathFlag,

cmd/mist/main.go

@@ -69,7 +69,7 @@ func init() {
 		assetPathFlag,
 		rpcCorsFlag,
 
-		utils.BootNodesFlag,
+		utils.BootnodesFlag,
 		utils.DataDirFlag,
 		utils.ListenPortFlag,
 		utils.LogFileFlag,

cmd/mist/ui_lib.go

@@ -104,7 +104,7 @@ func (ui *UiLib) Connect(button qml.Object) {
 }
 
 func (ui *UiLib) ConnectToPeer(nodeURL string) {
-	if err := ui.eth.TrustPeer(nodeURL); err != nil {
+	if err := ui.eth.AddPeer(nodeURL); err != nil {
 		guilogger.Infoln("TrustPeer error: " + err.Error())
 	}
 }

cmd/utils/flags.go

@@ -202,7 +202,7 @@ var (
 		Usage: "Network listening port",
 		Value: 30303,
 	}
-	BootNodesFlag = cli.StringFlag{
+	BootnodesFlag = cli.StringFlag{
 		Name:  "bootnodes",
 		Usage: "Space-separated enode URLs for p2p discovery bootstrap",
 		Value: "",
@@ -292,7 +292,7 @@ func MakeEthConfig(clientID, version string, ctx *cli.Context) *eth.Config {
 		NodeKey:            GetNodeKey(ctx),
 		Shh:                ctx.GlobalBool(WhisperEnabledFlag.Name),
 		Dial:               true,
-		BootNodes:          ctx.GlobalString(BootNodesFlag.Name),
+		BootNodes:          ctx.GlobalString(BootnodesFlag.Name),
 	}
 }
 

eth/backend.go

@@ -108,7 +108,6 @@ func (cfg *Config) parseTrustedNodes() []*discover.Node {
 	// Short circuit if no trusted node config is present
 	path := filepath.Join(cfg.DataDir, trustedNodes)
 	if _, err := os.Stat(path); err != nil {
-		fmt.Println("nodes", nil)
 		return nil
 	}
 	// Load the trusted nodes from the config file
@@ -122,7 +121,6 @@ func (cfg *Config) parseTrustedNodes() []*discover.Node {
 		glog.V(logger.Error).Infof("Failed to load trusted nodes: %v", err)
 		return nil
 	}
-	fmt.Println("nodes", nodelist)
 	// Interpret the list as a discovery node array
 	var nodes []*discover.Node
 	for _, url := range nodelist {
@@ -486,13 +484,15 @@ func (s *Ethereum) StartForTest() {
 	s.txPool.Start()
 }
 
-// TrustPeer injects a new node into the list of privileged nodes.
-func (self *Ethereum) TrustPeer(nodeURL string) error {
+// AddPeer connects to the given node and maintains the connection until the
+// server is shut down. If the connection fails for any reason, the server will
+// attempt to reconnect the peer.
+func (self *Ethereum) AddPeer(nodeURL string) error {
 	n, err := discover.ParseNode(nodeURL)
 	if err != nil {
 		return fmt.Errorf("invalid node URL: %v", err)
 	}
-	self.net.TrustPeer(n)
+	self.net.AddPeer(n)
 	return nil
 }
 

p2p/server.go

@@ -100,10 +100,9 @@ type Server struct {
 
 	ourHandshake *protoHandshake
 
-	lock    sync.RWMutex // protects running and peers
-	running bool
-	peers   map[discover.NodeID]*Peer
-
+	lock      sync.RWMutex // protects running, peers and the trust fields
+	running   bool
+	peers     map[discover.NodeID]*Peer
 	trusts    map[discover.NodeID]*discover.Node // Map of currently trusted remote nodes
 	trustDial chan *discover.Node                // Dial request channel reserved for the trusted nodes
 
@@ -138,8 +137,10 @@ func (srv *Server) PeerCount() int {
 	return n
 }
 
-// TrustPeer inserts a node into the list of privileged nodes.
-func (srv *Server) TrustPeer(node *discover.Node) {
+// AddPeer connects to the given node and maintains the connection until the
+// server is shut down. If the connection fails for any reason, the server will
+// attempt to reconnect the peer.
+func (srv *Server) AddPeer(node *discover.Node) {
 	srv.lock.Lock()
 	defer srv.lock.Unlock()
 
@@ -246,7 +247,7 @@ func (srv *Server) Start() (err error) {
 		glog.V(logger.Warn).Infoln("I will be kind-of useless, neither dialing nor listening.")
 	}
 	// maintain the trusted peers
-	go srv.trustLoop()
+	go srv.trustedNodesLoop()
 
 	srv.running = true
 	return nil
@@ -341,16 +342,13 @@ func (srv *Server) listenLoop() {
 	}
 }
 
-// trustLoop is responsible for periodically checking that trusted connections
-// are actually live, and requests dialing if not.
-func (srv *Server) trustLoop() {
-	// Create a ticker for verifying trusted connections
+// trustedNodesLoop is responsible for periodically checking that trusted
+// connections are actually live, and requests dialing if not.
+func (srv *Server) trustedNodesLoop() {
 	tick := time.Tick(trustedPeerCheckInterval)
-
 	for {
 		select {
 		case <-srv.quit:
-			// Termination requested, simple return
 			return
 
 		case <-tick:
@@ -369,10 +367,7 @@ func (srv *Server) trustLoop() {
 				glog.V(logger.Debug).Infof("Dialing trusted peer %v", node)
 				select {
 				case srv.trustDial <- node:
-					// Ok, dialing
-
 				case <-srv.quit:
-					// Terminating, return
 					return
 				}
 			}
@@ -547,16 +542,12 @@ func (srv *Server) checkPeer(id discover.NodeID) (bool, DiscReason) {
 	switch {
 	case !srv.running:
 		return false, DiscQuitting
-
 	case !trusted && len(srv.peers) >= srv.MaxPeers:
 		return false, DiscTooManyPeers
-
 	case srv.peers[id] != nil:
 		return false, DiscAlreadyConnected
-
 	case id == srv.ntab.Self().ID:
 		return false, DiscSelf
-
 	default:
 		return true, 0
 	}

p2p/server_test.go

@@ -260,7 +260,7 @@ func TestServerTrustedPeers(t *testing.T) {
 	trusted := &discover.Node{
 		ID: discover.PubkeyID(&key.PublicKey),
 	}
-	server.TrustPeer(trusted)
+	server.AddPeer(trusted)
 
 	conn, err := dialer.Dial("tcp", server.ListenAddr)
 	if err != nil {