Start discovery after sending "node.ready" signal (#1333)

We would like to optimize sending "node.ready" signal and do it as soon as possible. The peers discovery protocol can be started after the signal is sent.

api/backend.go

@@ -135,7 +135,13 @@ func (b *StatusBackend) startNode(config *params.NodeConfig) (err error) {
 	services := []gethnode.ServiceConstructor{}
 	services = appendIf(config.UpstreamConfig.Enabled, services, b.rpcFiltersService())
 
-	if err = b.statusNode.Start(config, services...); err != nil {
+	if err = b.statusNode.StartWithOptions(config, node.StartOptions{
+		Services: services,
+		// The peers discovery protocols are started manually after
+		// `node.ready` signal is sent.
+		// It was discussed in https://github.com/status-im/status-go/pull/1333.
+		StartDiscovery: false,
+	}); err != nil {
 		return
 	}
 	signal.SendNodeStarted()
@@ -166,6 +172,10 @@ func (b *StatusBackend) startNode(config *params.NodeConfig) (err error) {
 
 	signal.SendNodeReady()
 
+	if err := b.statusNode.StartDiscovery(); err != nil {
+		return err
+	}
+
 	return nil
 }
 

node/status_node.go

@@ -43,6 +43,7 @@ var (
 	ErrNoRunningNode          = errors.New("there is no running node")
 	ErrAccountKeyStoreMissing = errors.New("account key store is not set")
 	ErrServiceUnknown         = errors.New("service unknown")
+	ErrDiscoveryRunning       = errors.New("discovery is already running")
 )
 
 // StatusNode abstracts contained geth node and provides helper methods to
@@ -98,28 +99,24 @@ func (n *StatusNode) Server() *p2p.Server {
 	return n.gethNode.Server()
 }
 
-func (n *StatusNode) startWithDB(config *params.NodeConfig, db *leveldb.DB, services []node.ServiceConstructor) error {
+// Start starts current StatusNode, failing if it's already started.
-	if err := n.createNode(config, db); err != nil {
+// It accepts a list of services that should be added to the node.
-		return err
+func (n *StatusNode) Start(config *params.NodeConfig, services ...node.ServiceConstructor) error {
-	}
+	return n.StartWithOptions(config, StartOptions{
-	n.config = config
+		Services:       services,
-
+		StartDiscovery: true,
-	if err := n.start(services); err != nil {
+	})
-		return err
-	}
-
-	if err := n.setupRPCClient(); err != nil {
-		return err
-	}
-
-	if n.discoveryEnabled() {
-		return n.startDiscovery()
-	}
-	return nil
 }
 
-// Start starts current StatusNode, failing if it's already started.
+// StartOptions allows to control some parameters of Start() method.
-func (n *StatusNode) Start(config *params.NodeConfig, services ...node.ServiceConstructor) error {
+type StartOptions struct {
+	Services       []node.ServiceConstructor
+	StartDiscovery bool
+}
+
+// StartWithOptions starts current StatusNode, failing if it's already started.
+// It takes some options that allows to further configure starting process.
+func (n *StatusNode) StartWithOptions(config *params.NodeConfig, options StartOptions) error {
 	n.mu.Lock()
 	defer n.mu.Unlock()
 
@@ -137,7 +134,12 @@ func (n *StatusNode) Start(config *params.NodeConfig, services ...node.ServiceCo
 
 	n.db = db
 
-	err = n.startWithDB(config, db, services)
+	err = n.startWithDB(config, db, options.Services)
+
+	// continue only if there was no error when starting node with a db
+	if err == nil && options.StartDiscovery && n.discoveryEnabled() {
+		err = n.startDiscovery()
+	}
 
 	if err != nil {
 		if dberr := db.Close(); dberr != nil {
@@ -150,6 +152,23 @@ func (n *StatusNode) Start(config *params.NodeConfig, services ...node.ServiceCo
 	return nil
 }
 
+func (n *StatusNode) startWithDB(config *params.NodeConfig, db *leveldb.DB, services []node.ServiceConstructor) error {
+	if err := n.createNode(config, db); err != nil {
+		return err
+	}
+	n.config = config
+
+	if err := n.start(services); err != nil {
+		return err
+	}
+
+	if err := n.setupRPCClient(); err != nil {
+		return err
+	}
+
+	return nil
+}
+
 func (n *StatusNode) createNode(config *params.NodeConfig, db *leveldb.DB) (err error) {
 	n.gethNode, err = MakeNode(config, db)
 	return err
@@ -235,7 +254,23 @@ func (n *StatusNode) startRendezvous() (discovery.Discovery, error) {
 	return discovery.NewRendezvous(maddrs, n.gethNode.Server().PrivateKey, node)
 }
 
+// StartDiscovery starts the peers discovery protocols depending on the node config.
+func (n *StatusNode) StartDiscovery() error {
+	n.mu.Lock()
+	defer n.mu.Unlock()
+
+	if n.discoveryEnabled() {
+		return n.startDiscovery()
+	}
+
+	return nil
+}
+
 func (n *StatusNode) startDiscovery() error {
+	if n.isDiscoveryRunning() {
+		return ErrDiscoveryRunning
+	}
+
 	discoveries := []discovery.Discovery{}
 	if !n.config.NoDiscovery {
 		discoveries = append(discoveries, discovery.NewDiscV5(
@@ -330,6 +365,10 @@ func (n *StatusNode) stop() error {
 	return nil
 }
 
+func (n *StatusNode) isDiscoveryRunning() bool {
+	return n.register != nil || n.peerPool != nil || n.discovery != nil
+}
+
 func (n *StatusNode) stopDiscovery() error {
 	n.register.Stop()
 	n.peerPool.Stop()

peers/peerpool_test.go

@@ -15,7 +15,6 @@ import (
 	"github.com/ethereum/go-ethereum/p2p/enode"
 	lcrypto "github.com/libp2p/go-libp2p-crypto"
 	ma "github.com/multiformats/go-multiaddr"
-	"github.com/status-im/whisper/whisperv6"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	"github.com/stretchr/testify/suite"
@@ -26,9 +25,7 @@ import (
 	"github.com/status-im/status-go/discovery"
 	"github.com/status-im/status-go/params"
 	"github.com/status-im/status-go/signal"
-
+	whisper "github.com/status-im/whisper/whisperv6"
-	// to access logs in the test with `-log` flag
-	_ "github.com/status-im/status-go/t/utils"
 )
 
 type PeerPoolSimulationSuite struct {
@@ -75,7 +72,7 @@ func (s *PeerPoolSimulationSuite) SetupTest() {
 	s.discovery = make([]discovery.Discovery, 3)
 	for i := range s.peers {
 		key, _ := crypto.GenerateKey()
-		whisper := whisperv6.New(nil)
+		whisper := whisper.New(nil)
 		peer := &p2p.Server{
 			Config: p2p.Config{
 				MaxPeers:         10,