From b5cdd54be20836a933402307d82d894ed9276252 Mon Sep 17 00:00:00 2001 From: devopsbo3 <69951731+devopsbo3@users.noreply.github.com> Date: Fri, 10 Nov 2023 12:27:53 -0600 Subject: [PATCH] Revert "cmd/utils, p2p: clean up discovery setup (#27518)" This reverts commit 899a6560ec7b59933fbb629c2d500031e26a7d3a. --- cmd/geth/les_test.go | 4 +- cmd/geth/main.go | 2 - cmd/utils/flags.go | 33 +++++------- cmd/utils/flags_legacy.go | 21 ++------ p2p/server.go | 109 ++++++++++++++++++-------------------- 5 files changed, 70 insertions(+), 99 deletions(-) diff --git a/cmd/geth/les_test.go b/cmd/geth/les_test.go index 746f12c765..607b454ead 100644 --- a/cmd/geth/les_test.go +++ b/cmd/geth/les_test.go @@ -146,13 +146,13 @@ func startLightServer(t *testing.T) *gethrpc { t.Logf("Importing keys to geth") runGeth(t, "account", "import", "--datadir", datadir, "--password", "./testdata/password.txt", "--lightkdf", "./testdata/key.prv").WaitExit() account := "0x02f0d131f1f97aef08aec6e3291b957d9efe7105" - server := startGethWithIpc(t, "lightserver", "--allow-insecure-unlock", "--datadir", datadir, "--password", "./testdata/password.txt", "--unlock", account, "--miner.etherbase=0x02f0d131f1f97aef08aec6e3291b957d9efe7105", "--mine", "--light.serve=100", "--light.maxpeers=1", "--discv4=false", "--nat=extip:127.0.0.1", "--verbosity=4") + server := startGethWithIpc(t, "lightserver", "--allow-insecure-unlock", "--datadir", datadir, "--password", "./testdata/password.txt", "--unlock", account, "--miner.etherbase=0x02f0d131f1f97aef08aec6e3291b957d9efe7105", "--mine", "--light.serve=100", "--light.maxpeers=1", "--nodiscover", "--nat=extip:127.0.0.1", "--verbosity=4") return server } func startClient(t *testing.T, name string) *gethrpc { datadir := initGeth(t) - return startGethWithIpc(t, name, "--datadir", datadir, "--discv4=false", "--syncmode=light", "--nat=extip:127.0.0.1", "--verbosity=4") + return startGethWithIpc(t, name, "--datadir", datadir, "--nodiscover", "--syncmode=light", "--nat=extip:127.0.0.1", "--verbosity=4") } func TestPriorityClient(t *testing.T) { diff --git a/cmd/geth/main.go b/cmd/geth/main.go index b9584e70b3..42a05f4d1a 100644 --- a/cmd/geth/main.go +++ b/cmd/geth/main.go @@ -122,9 +122,7 @@ var ( utils.MinerNewPayloadTimeout, utils.NATFlag, utils.NoDiscoverFlag, - utils.DiscoveryV4Flag, utils.DiscoveryV5Flag, - utils.LegacyDiscoveryV5Flag, utils.NetrestrictFlag, utils.NodeKeyFileFlag, utils.NodeKeyHexFlag, diff --git a/cmd/utils/flags.go b/cmd/utils/flags.go index feceb99d3e..525abf042d 100644 --- a/cmd/utils/flags.go +++ b/cmd/utils/flags.go @@ -253,6 +253,11 @@ var ( Usage: "Comma separated block number-to-hash mappings to require for peering (=)", Category: flags.EthCategory, } + LegacyWhitelistFlag = &cli.StringFlag{ + Name: "whitelist", + Usage: "Comma separated block number-to-hash mappings to enforce (=) (deprecated in favor of --eth.requiredblocks)", + Category: flags.DeprecatedCategory, + } BloomFilterSizeFlag = &cli.Uint64Flag{ Name: "bloomfilter.size", Usage: "Megabytes of memory allocated to bloom-filter for pruning", @@ -765,16 +770,8 @@ var ( Usage: "Disables the peer discovery mechanism (manual peer addition)", Category: flags.NetworkingCategory, } - DiscoveryV4Flag = &cli.BoolFlag{ - Name: "discovery.v4", - Aliases: []string{"discv4"}, - Usage: "Enables the V4 discovery mechanism", - Category: flags.NetworkingCategory, - Value: true, - } DiscoveryV5Flag = &cli.BoolFlag{ - Name: "discovery.v5", - Aliases: []string{"discv5"}, + Name: "v5disc", Usage: "Enables the experimental RLPx V5 (Topic Discovery) mechanism", Category: flags.NetworkingCategory, } @@ -1364,17 +1361,13 @@ func SetP2PConfig(ctx *cli.Context, cfg *p2p.Config) { cfg.NoDiscovery = true } - // Disallow --nodiscover when used in conjunction with light mode. - if (lightClient || lightServer) && ctx.Bool(NoDiscoverFlag.Name) { - Fatalf("Cannot use --" + NoDiscoverFlag.Name + " in light client or light server mode") - } - CheckExclusive(ctx, DiscoveryV4Flag, NoDiscoverFlag) - CheckExclusive(ctx, DiscoveryV5Flag, NoDiscoverFlag) - cfg.DiscoveryV4 = ctx.Bool(DiscoveryV4Flag.Name) - cfg.DiscoveryV5 = ctx.Bool(DiscoveryV5Flag.Name) - - // If we're running a light client or server, force enable the v5 peer discovery. - if lightClient || lightServer { + // if we're running a light client or server, force enable the v5 peer discovery + // unless it is explicitly disabled with --nodiscover note that explicitly specifying + // --v5disc overrides --nodiscover, in which case the later only disables v4 discovery + forceV5Discovery := (lightClient || lightServer) && !ctx.Bool(NoDiscoverFlag.Name) + if ctx.IsSet(DiscoveryV5Flag.Name) { + cfg.DiscoveryV5 = ctx.Bool(DiscoveryV5Flag.Name) + } else if forceV5Discovery { cfg.DiscoveryV5 = true } diff --git a/cmd/utils/flags_legacy.go b/cmd/utils/flags_legacy.go index cf51b51936..b554deba8e 100644 --- a/cmd/utils/flags_legacy.go +++ b/cmd/utils/flags_legacy.go @@ -33,40 +33,27 @@ var ShowDeprecated = &cli.Command{ var DeprecatedFlags = []cli.Flag{ NoUSBFlag, - LegacyWhitelistFlag, CacheTrieJournalFlag, CacheTrieRejournalFlag, - LegacyDiscoveryV5Flag, } var ( - // Deprecated May 2020, shown in aliased flags section + // (Deprecated May 2020, shown in aliased flags section) NoUSBFlag = &cli.BoolFlag{ Name: "nousb", Usage: "Disables monitoring for and managing USB hardware wallets (deprecated)", Category: flags.DeprecatedCategory, } - // Deprecated March 2022 - LegacyWhitelistFlag = &cli.StringFlag{ - Name: "whitelist", - Usage: "Comma separated block number-to-hash mappings to enforce (=) (deprecated in favor of --eth.requiredblocks)", - Category: flags.DeprecatedCategory, - } - // Deprecated July 2023 + // (Deprecated June 2023, shown in aliased flags section) CacheTrieJournalFlag = &cli.StringFlag{ Name: "cache.trie.journal", Usage: "Disk journal directory for trie cache to survive node restarts", - Category: flags.DeprecatedCategory, + Category: flags.PerfCategory, } CacheTrieRejournalFlag = &cli.DurationFlag{ Name: "cache.trie.rejournal", Usage: "Time interval to regenerate the trie cache journal", - Category: flags.DeprecatedCategory, - } - LegacyDiscoveryV5Flag = &cli.BoolFlag{ - Name: "v5disc", - Usage: "Enables the experimental RLPx V5 (Topic Discovery) mechanism (deprecated, use --discv5 instead)", - Category: flags.DeprecatedCategory, + Category: flags.PerfCategory, } ) diff --git a/p2p/server.go b/p2p/server.go index bd0fd70950..8c417635e6 100644 --- a/p2p/server.go +++ b/p2p/server.go @@ -93,9 +93,6 @@ type Config struct { // Disabling is useful for protocol debugging (manual topology). NoDiscovery bool - // DiscoveryV4 specifies whether V4 discovery should be started. - DiscoveryV4 bool `toml:",omitempty"` - // DiscoveryV5 specifies whether the new topic-discovery based V5 discovery // protocol should be started or not. DiscoveryV5 bool `toml:",omitempty"` @@ -545,28 +542,57 @@ func (srv *Server) setupLocalNode() error { func (srv *Server) setupDiscovery() error { srv.discmix = enode.NewFairMix(discmixTimeout) + // Add protocol-specific discovery sources. + added := make(map[string]bool) + for _, proto := range srv.Protocols { + if proto.DialCandidates != nil && !added[proto.Name] { + srv.discmix.AddSource(proto.DialCandidates) + added[proto.Name] = true + } + } + // Don't listen on UDP endpoint if DHT is disabled. - if srv.NoDiscovery { + if srv.NoDiscovery && !srv.DiscoveryV5 { return nil } - conn, err := srv.setupUDPListening() + + listenAddr := srv.ListenAddr + + // Use an alternate listening address for UDP if + // a custom discovery address is configured. + if srv.DiscAddr != "" { + listenAddr = srv.DiscAddr + } + + addr, err := net.ResolveUDPAddr("udp", listenAddr) if err != nil { return err } - - var ( - sconn discover.UDPConn = conn - unhandled chan discover.ReadPacket - ) - // If both versions of discovery are running, setup a shared - // connection, so v5 can read unhandled messages from v4. - if srv.DiscoveryV4 && srv.DiscoveryV5 { - unhandled = make(chan discover.ReadPacket, 100) - sconn = &sharedUDPConn{conn, unhandled} + conn, err := net.ListenUDP("udp", addr) + if err != nil { + return err } + realaddr := conn.LocalAddr().(*net.UDPAddr) + srv.log.Debug("UDP listener up", "addr", realaddr) + if srv.NAT != nil { + if !realaddr.IP.IsLoopback() { + srv.loopWG.Add(1) + go func() { + nat.Map(srv.NAT, srv.quit, "udp", realaddr.Port, realaddr.Port, "ethereum discovery") + srv.loopWG.Done() + }() + } + } + srv.localnode.SetFallbackUDP(realaddr.Port) - // Start discovery services. - if srv.DiscoveryV4 { + // Discovery V4 + var unhandled chan discover.ReadPacket + var sconn *sharedUDPConn + if !srv.NoDiscovery { + if srv.DiscoveryV5 { + unhandled = make(chan discover.ReadPacket, 100) + sconn = &sharedUDPConn{conn, unhandled} + } cfg := discover.Config{ PrivateKey: srv.PrivateKey, NetRestrict: srv.NetRestrict, @@ -581,6 +607,8 @@ func (srv *Server) setupDiscovery() error { srv.ntab = ntab srv.discmix.AddSource(ntab.RandomNodes()) } + + // Discovery V5 if srv.DiscoveryV5 { cfg := discover.Config{ PrivateKey: srv.PrivateKey, @@ -588,20 +616,16 @@ func (srv *Server) setupDiscovery() error { Bootnodes: srv.BootstrapNodesV5, Log: srv.log, } - srv.DiscV5, err = discover.ListenV5(sconn, srv.localnode, cfg) + var err error + if sconn != nil { + srv.DiscV5, err = discover.ListenV5(sconn, srv.localnode, cfg) + } else { + srv.DiscV5, err = discover.ListenV5(conn, srv.localnode, cfg) + } if err != nil { return err } } - - // Add protocol-specific discovery sources. - added := make(map[string]bool) - for _, proto := range srv.Protocols { - if proto.DialCandidates != nil && !added[proto.Name] { - srv.discmix.AddSource(proto.DialCandidates) - added[proto.Name] = true - } - } return nil } @@ -672,37 +696,6 @@ func (srv *Server) setupListening() error { return nil } -func (srv *Server) setupUDPListening() (*net.UDPConn, error) { - listenAddr := srv.ListenAddr - - // Use an alternate listening address for UDP if - // a custom discovery address is configured. - if srv.DiscAddr != "" { - listenAddr = srv.DiscAddr - } - addr, err := net.ResolveUDPAddr("udp", listenAddr) - if err != nil { - return nil, err - } - conn, err := net.ListenUDP("udp", addr) - if err != nil { - return nil, err - } - realaddr := conn.LocalAddr().(*net.UDPAddr) - srv.log.Debug("UDP listener up", "addr", realaddr) - if srv.NAT != nil { - if !realaddr.IP.IsLoopback() { - srv.loopWG.Add(1) - go func() { - nat.Map(srv.NAT, srv.quit, "udp", realaddr.Port, realaddr.Port, "ethereum discovery") - srv.loopWG.Done() - }() - } - } - srv.localnode.SetFallbackUDP(realaddr.Port) - return conn, nil -} - // doPeerOp runs fn on the main loop. func (srv *Server) doPeerOp(fn peerOpFunc) { select {