Oneeman/cherry pick discovery bootnode fix

cmd/devp2p/crawl.go

@@ -20,14 +20,13 @@ import (
 	"time"
 
 	"github.com/ethereum/go-ethereum/log"
-	"github.com/ethereum/go-ethereum/p2p/discover"
 	"github.com/ethereum/go-ethereum/p2p/enode"
 )
 
 type crawler struct {
 	input     nodeSet
 	output    nodeSet
-	disc      *discover.UDPv4
+	disc      resolver
 	iters     []enode.Iterator
 	inputIter enode.Iterator
 	ch        chan *enode.Node
@@ -37,7 +36,11 @@ type crawler struct {
 	revalidateInterval time.Duration
 }
 
-func newCrawler(input nodeSet, disc *discover.UDPv4, iters ...enode.Iterator) *crawler {
+type resolver interface {
+	RequestENR(*enode.Node) (*enode.Node, error)
+}
+
+func newCrawler(input nodeSet, disc resolver, iters ...enode.Iterator) *crawler {
 	c := &crawler{
 		input:     input,
 		output:    make(nodeSet, len(input)),

cmd/devp2p/discv4cmd.go

@@ -83,6 +83,18 @@ var (
 		Name:  "bootnodes",
 		Usage: "Comma separated nodes used for bootstrapping",
 	}
+	nodekeyFlag = cli.StringFlag{
+		Name:  "nodekey",
+		Usage: "Hex-encoded node key",
+	}
+	nodedbFlag = cli.StringFlag{
+		Name:  "nodedb",
+		Usage: "Nodes database location",
+	}
+	listenAddrFlag = cli.StringFlag{
+		Name:  "addr",
+		Usage: "Listening address",
+	}
 	crawlTimeoutFlag = cli.DurationFlag{
 		Name:  "timeout",
 		Usage: "Time limit for the crawl.",
@@ -180,56 +192,75 @@ func discv4Crawl(ctx *cli.Context) error {
 	return nil
 }
 
-func parseBootnodes(ctx *cli.Context) ([]*enode.Node, error) {
-	s := utils.GetBootstrapNodes(ctx)
-	if ctx.IsSet(bootnodesFlag.Name) {
-		s = strings.Split(ctx.String(bootnodesFlag.Name), ",")
-	}
-	nodes := make([]*enode.Node, len(s))
-	var err error
-	for i, record := range s {
-		nodes[i], err = parseNode(record)
-		if err != nil {
-			return nil, fmt.Errorf("invalid bootstrap node: %v", err)
-		}
-	}
-	return nodes, nil
-}
-
 // startV4 starts an ephemeral discovery V4 node.
 func startV4(ctx *cli.Context) *discover.UDPv4 {
 	networkId := ctx.GlobalUint64(networkIdFlag.Name)
-	socket, ln, cfg, err := listen(networkId)
+	ln, config := makeDiscoveryConfig(ctx, networkId)
+	socket := listen(ln, ctx.String(listenAddrFlag.Name))
+	disc, err := discover.ListenV4(socket, ln, config)
 	if err != nil {
 		exit(err)
 	}
+	return disc
+}
+
+func makeDiscoveryConfig(ctx *cli.Context, networkId uint64) (*enode.LocalNode, discover.Config) {
+	var cfg discover.Config
+
+	if ctx.IsSet(nodekeyFlag.Name) {
+		key, err := crypto.HexToECDSA(ctx.String(nodekeyFlag.Name))
+		if err != nil {
+			exit(fmt.Errorf("-%s: %v", nodekeyFlag.Name, err))
+		}
+		cfg.PrivateKey = key
+	} else {
+		cfg.PrivateKey, _ = crypto.GenerateKey()
+	}
+
 	if commandHasFlag(ctx, bootnodesFlag) {
 		bn, err := parseBootnodes(ctx)
 		if err != nil {
 			exit(err)
 		}
 		cfg.Bootnodes = bn
 	}
-	disc, err := discover.ListenV4(socket, ln, cfg)
+
+	dbpath := ctx.String(nodedbFlag.Name)
+	db, err := enode.OpenDB(dbpath)
 	if err != nil {
 		exit(err)
 	}
-	return disc
-}
-
-func listen(networkId uint64) (*net.UDPConn, *enode.LocalNode, discover.Config, error) {
-	var cfg discover.Config
-	cfg.PrivateKey, _ = crypto.GenerateKey()
-	db, _ := enode.OpenDB("")
 	ln := enode.NewLocalNode(db, cfg.PrivateKey, networkId)
+	return ln, cfg
+}
 
-	socket, err := net.ListenUDP("udp4", &net.UDPAddr{IP: net.IP{0, 0, 0, 0}})
+func listen(ln *enode.LocalNode, addr string) *net.UDPConn {
+	if addr == "" {
+		addr = "0.0.0.0:0"
+	}
+	socket, err := net.ListenPacket("udp4", addr)
 	if err != nil {
-		db.Close()
-		return nil, nil, cfg, err
+		exit(err)
 	}
-	addr := socket.LocalAddr().(*net.UDPAddr)
+	usocket := socket.(*net.UDPConn)
+	uaddr := socket.LocalAddr().(*net.UDPAddr)
 	ln.SetFallbackIP(net.IP{127, 0, 0, 1})
-	ln.SetFallbackUDP(addr.Port)
-	return socket, ln, cfg, nil
+	ln.SetFallbackUDP(uaddr.Port)
+	return usocket
+}
+
+func parseBootnodes(ctx *cli.Context) ([]*enode.Node, error) {
+	s := utils.GetBootstrapNodes(ctx)
+	if ctx.IsSet(bootnodesFlag.Name) {
+		s = strings.Split(ctx.String(bootnodesFlag.Name), ",")
+	}
+	nodes := make([]*enode.Node, len(s))
+	var err error
+	for i, record := range s {
+		nodes[i], err = parseNode(record)
+		if err != nil {
+			return nil, fmt.Errorf("invalid bootstrap node: %v", err)
+		}
+	}
+	return nodes, nil
 }

cmd/devp2p/discv5cmd.go

@@ -0,0 +1,124 @@
+// Copyright 2019 The go-ethereum Authors
+// This file is part of go-ethereum.
+//
+// go-ethereum is free software: you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+//
+// go-ethereum is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+// GNU General Public License for more details.
+//
+// You should have received a copy of the GNU General Public License
+// along with go-ethereum. If not, see <http://www.gnu.org/licenses/>.
+
+package main
+
+import (
+	"fmt"
+	"time"
+
+	"github.com/ethereum/go-ethereum/common"
+	"github.com/ethereum/go-ethereum/p2p/discover"
+	"gopkg.in/urfave/cli.v1"
+)
+
+var (
+	discv5Command = cli.Command{
+		Name:  "discv5",
+		Usage: "Node Discovery v5 tools",
+		Subcommands: []cli.Command{
+			discv5PingCommand,
+			discv5ResolveCommand,
+			discv5CrawlCommand,
+			discv5ListenCommand,
+		},
+	}
+	discv5PingCommand = cli.Command{
+		Name:   "ping",
+		Usage:  "Sends ping to a node",
+		Action: discv5Ping,
+	}
+	discv5ResolveCommand = cli.Command{
+		Name:   "resolve",
+		Usage:  "Finds a node in the DHT",
+		Action: discv5Resolve,
+		Flags:  []cli.Flag{bootnodesFlag},
+	}
+	discv5CrawlCommand = cli.Command{
+		Name:   "crawl",
+		Usage:  "Updates a nodes.json file with random nodes found in the DHT",
+		Action: discv5Crawl,
+		Flags:  []cli.Flag{bootnodesFlag, crawlTimeoutFlag},
+	}
+	discv5ListenCommand = cli.Command{
+		Name:   "listen",
+		Usage:  "Runs a node",
+		Action: discv5Listen,
+		Flags: []cli.Flag{
+			bootnodesFlag,
+			nodekeyFlag,
+			nodedbFlag,
+			listenAddrFlag,
+		},
+	}
+)
+
+func discv5Ping(ctx *cli.Context) error {
+	n := getNodeArg(ctx)
+	disc := startV5(ctx)
+	defer disc.Close()
+
+	fmt.Println(disc.Ping(n))
+	return nil
+}
+
+func discv5Resolve(ctx *cli.Context) error {
+	n := getNodeArg(ctx)
+	disc := startV5(ctx)
+	defer disc.Close()
+
+	fmt.Println(disc.Resolve(n))
+	return nil
+}
+
+func discv5Crawl(ctx *cli.Context) error {
+	if ctx.NArg() < 1 {
+		return fmt.Errorf("need nodes file as argument")
+	}
+	nodesFile := ctx.Args().First()
+	var inputSet nodeSet
+	if common.FileExist(nodesFile) {
+		inputSet = loadNodesJSON(nodesFile)
+	}
+
+	disc := startV5(ctx)
+	defer disc.Close()
+	c := newCrawler(inputSet, disc, disc.RandomNodes())
+	c.revalidateInterval = 10 * time.Minute
+	output := c.run(ctx.Duration(crawlTimeoutFlag.Name))
+	writeNodesJSON(nodesFile, output)
+	return nil
+}
+
+func discv5Listen(ctx *cli.Context) error {
+	disc := startV5(ctx)
+	defer disc.Close()
+
+	fmt.Println(disc.Self())
+	select {}
+}
+
+// startV5 starts an ephemeral discovery v5 node.
+func startV5(ctx *cli.Context) *discover.UDPv5 {
+	networkId := ctx.GlobalUint64(networkIdFlag.Name)
+	ln, config := makeDiscoveryConfig(ctx, networkId)
+	socket := listen(ln, ctx.String(listenAddrFlag.Name))
+	disc, err := discover.ListenV5(socket, ln, config)
+	if err != nil {
+		exit(err)
+	}
+	return disc
+}

cmd/devp2p/main.go

@@ -59,6 +59,7 @@ func init() {
 	app.Commands = []cli.Command{
 		enrdumpCommand,
 		discv4Command,
+		discv5Command,
 		dnsCommand,
 		nodesetCommand,
 	}

p2p/discover/common.go

@@ -20,8 +20,10 @@ import (
 	"crypto/ecdsa"
 	"net"
 
+	"github.com/ethereum/go-ethereum/common/mclock"
 	"github.com/ethereum/go-ethereum/log"
 	"github.com/ethereum/go-ethereum/p2p/enode"
+	"github.com/ethereum/go-ethereum/p2p/enr"
 	"github.com/ethereum/go-ethereum/p2p/netutil"
 )
 
@@ -44,6 +46,21 @@ type Config struct {
 	Unhandled        chan<- ReadPacket // unhandled packets are sent on this channel
 	Log              log.Logger        // if set, log messages go here
 	PingIPFromPacket bool
+	ValidSchemes     enr.IdentityScheme // allowed identity schemes
+	Clock            mclock.Clock
+}
+
+func (cfg Config) withDefaults() Config {
+	if cfg.Log == nil {
+		cfg.Log = log.Root()
+	}
+	if cfg.ValidSchemes == nil {
+		cfg.ValidSchemes = enode.ValidSchemes
+	}
+	if cfg.Clock == nil {
+		cfg.Clock = mclock.System{}
+	}
+	return cfg
 }
 
 // ListenUDP starts listening for discovery packets on the given UDP socket.
@@ -52,8 +69,15 @@ func ListenUDP(c UDPConn, ln *enode.LocalNode, cfg Config) (*UDPv4, error) {
 }
 
 // ReadPacket is a packet that couldn't be handled. Those packets are sent to the unhandled
-// channel if configured. This is exported for internal use, do not use this type.
+// channel if configured.
 type ReadPacket struct {
 	Data []byte
 	Addr *net.UDPAddr
 }
+
+func min(x, y int) int {
+	if x > y {
+		return y
+	}
+	return x
+}

p2p/discover/lookup.go

@@ -104,9 +104,7 @@ func (it *lookup) startQueries() bool {
 
 	// The first query returns nodes from the local table.
 	if it.queries == -1 {
-		it.tab.mutex.Lock()
-		closest := it.tab.closest(it.result.target, bucketSize, false)
-		it.tab.mutex.Unlock()
+		closest := it.tab.findnodeByID(it.result.target, bucketSize, false)
 		// Avoid finishing the lookup too quickly if table is empty. It'd be better to wait
 		// for the table to fill in this case, but there is no good mechanism for that
 		// yet.
@@ -150,11 +148,14 @@ func (it *lookup) query(n *node, reply chan<- []*node) {
 	} else if len(r) == 0 {
 		fails++
 		it.tab.db.UpdateFindFails(n.ID(), n.IP(), fails)
-		it.tab.log.Trace("Findnode failed", "id", n.ID(), "failcount", fails, "err", err)
-		if fails >= maxFindnodeFailures {
-			it.tab.log.Trace("Too many findnode failures, dropping", "id", n.ID(), "failcount", fails)
+		// Remove the node from the local table if it fails to return anything useful too
+		// many times, but only if there are enough other nodes in the bucket.
+		dropped := false
+		if fails >= maxFindnodeFailures && it.tab.bucketLen(n.ID()) >= bucketSize/2 {
+			dropped = true
 			it.tab.delete(n)
 		}
+		it.tab.log.Trace("FINDNODE failed", "id", n.ID(), "failcount", fails, "dropped", dropped, "err", err)
 	} else if fails > 0 {
 		// Reset failure counter because it counts _consecutive_ failures.
 		it.tab.db.UpdateFindFails(n.ID(), n.IP(), 0)