From 701591b403a8bae8c1dfb648a49d587968ff0c6a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C3=A9ter=20Szil=C3=A1gyi?= Date: Thu, 30 Apr 2015 16:15:29 +0300 Subject: [PATCH] cmd, eth, p2p: fix review issues enumerated by Felix --- cmd/geth/admin.go | 6 +++--- cmd/geth/main.go | 2 +- cmd/mist/main.go | 2 +- cmd/mist/ui_lib.go | 2 +- cmd/utils/flags.go | 4 ++-- eth/backend.go | 10 +++++----- p2p/server.go | 31 +++++++++++-------------------- p2p/server_test.go | 2 +- 8 files changed, 25 insertions(+), 34 deletions(-) diff --git a/cmd/geth/admin.go b/cmd/geth/admin.go index a07e694de..77510b8b8 100644 --- a/cmd/geth/admin.go +++ b/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() diff --git a/cmd/geth/main.go b/cmd/geth/main.go index 036ee2d0c..ef007051c 100644 --- a/cmd/geth/main.go +++ b/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, diff --git a/cmd/mist/main.go b/cmd/mist/main.go index 18fb919b4..1030d6ada 100644 --- a/cmd/mist/main.go +++ b/cmd/mist/main.go @@ -69,7 +69,7 @@ func init() { assetPathFlag, rpcCorsFlag, - utils.BootNodesFlag, + utils.BootnodesFlag, utils.DataDirFlag, utils.ListenPortFlag, utils.LogFileFlag, diff --git a/cmd/mist/ui_lib.go b/cmd/mist/ui_lib.go index e1a3aa254..a20205b03 100644 --- a/cmd/mist/ui_lib.go +++ b/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()) } } diff --git a/cmd/utils/flags.go b/cmd/utils/flags.go index 62b49cddc..c013510d8 100644 --- a/cmd/utils/flags.go +++ b/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), } } diff --git a/eth/backend.go b/eth/backend.go index c69e4a27a..11e5c25e8 100644 --- a/eth/backend.go +++ b/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 } diff --git a/p2p/server.go b/p2p/server.go index d8c5ecd77..dbb2e5f9e 100644 --- a/p2p/server.go +++ b/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 } diff --git a/p2p/server_test.go b/p2p/server_test.go index a79679ac1..3e3fd6cc0 100644 --- a/p2p/server_test.go +++ b/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 {