rpc: /dial_peers: only mark peers as persistent if flag is on (#3620)

## Description

also

    handle errors from DialPeersAsync
    remove nil addr from log msg
    fix TestPEXReactorDoesNotDisconnectFromPersistentPeerInSeedMode

This is a follow-up from
#3593 (review)

Fixes most of the #3617, except #3593 (comment)

## Commits

* rpc: /dial_peers: only mark peers as persistent if flag is on

also

- handle errors from DialPeersAsync
- remove nil addr from log msg
- fix TestPEXReactorDoesNotDisconnectFromPersistentPeerInSeedMode

This is a follow-up from
https://github.com/tendermint/tendermint/pull/3593#pullrequestreview-233556909

* remove a call to AddPersistentPeers

TestDialFail will trigger a reconnect

node/node.go

@@ -682,8 +682,10 @@ func (n *Node) OnStart() error {
 	}
 
 	// Always connect to persistent peers
-	// parsing errors are handled above by AddPersistentPeers
-	_ = n.sw.DialPeersAsync(splitAndTrimEmpty(n.config.P2P.PersistentPeers, ",", " "))
+	err = n.sw.DialPeersAsync(splitAndTrimEmpty(n.config.P2P.PersistentPeers, ",", " "))
+	if err != nil {
+		return errors.Wrap(err, "could not dial peers from persistent_peers field")
+	}
 
 	return nil
 }

p2p/pex/pex_reactor_test.go

@@ -330,7 +330,8 @@ func TestPEXReactorDoesNotDisconnectFromPersistentPeerInSeedMode(t *testing.T) {
 	require.Nil(t, err)
 	defer os.RemoveAll(dir) // nolint: errcheck
 
-	pexR, book := createReactor(&PEXReactorConfig{SeedMode: true, SeedDisconnectWaitPeriod: 1 * time.Millisecond})
+	pexRConfig := &PEXReactorConfig{SeedMode: true, SeedDisconnectWaitPeriod: 1 * time.Millisecond}
+	pexR, book := createReactor(pexRConfig)
 	defer teardownReactor(book)
 
 	sw := createSwitchAndAddReactors(pexR)
@@ -353,6 +354,9 @@ func TestPEXReactorDoesNotDisconnectFromPersistentPeerInSeedMode(t *testing.T) {
 	assert.Equal(t, 1, sw.Peers().Size())
 	assert.True(t, sw.Peers().Has(peerSwitch.NodeInfo().ID()))
 
+	// sleep for SeedDisconnectWaitPeriod
+	time.Sleep(pexRConfig.SeedDisconnectWaitPeriod + 1*time.Millisecond)
+
 	// 2. attemptDisconnects should not disconnect because the peer is persistent
 	pexR.attemptDisconnects()
 	assert.Equal(t, 1, sw.Peers().Size())

p2p/switch.go

@@ -308,7 +308,7 @@ func (sw *Switch) StopPeerForError(peer Peer, reason interface{}) {
 			addr, err = peer.NodeInfo().NetAddress()
 			if err != nil {
 				sw.Logger.Error("Wanted to reconnect to inbound peer, but self-reported address is wrong",
-					"peer", peer, "addr", addr, "err", err)
+					"peer", peer, "err", err)
 				return
 			}
 		}

p2p/switch_test.go

@@ -410,7 +410,6 @@ func TestSwitchReconnectsToOutboundPersistentPeer(t *testing.T) {
 
 	err = sw.DialPeerWithAddress(rp.Addr())
 	require.Nil(t, err)
-	time.Sleep(50 * time.Millisecond)
 	require.NotNil(t, sw.Peers().Get(rp.ID()))
 
 	p := sw.Peers().List()[0]
@@ -432,7 +431,7 @@ func TestSwitchReconnectsToOutboundPersistentPeer(t *testing.T) {
 	defer rp.Stop()
 
 	conf := config.DefaultP2PConfig()
-	conf.TestDialFail = true
+	conf.TestDialFail = true // will trigger a reconnect
 	err = sw.addOutboundPeerWithConfig(rp.Addr(), conf)
 	require.NotNil(t, err)
 	// DialPeerWithAddres - sw.peerConfig resets the dialer

rpc/core/net.go

@@ -196,11 +196,14 @@ func UnsafeDialPeers(ctx *rpctypes.Context, peers []string, persistent bool) (*c
 		return &ctypes.ResultDialPeers{}, errors.New("No peers provided")
 	}
 	logger.Info("DialPeers", "peers", peers, "persistent", persistent)
-	if err := p2pPeers.AddPersistentPeers(peers); err != nil {
+	if persistent {
+		if err := p2pPeers.AddPersistentPeers(peers); err != nil {
+			return &ctypes.ResultDialPeers{}, err
+		}
+	}
+	if err := p2pPeers.DialPeersAsync(peers); err != nil {
 		return &ctypes.ResultDialPeers{}, err
 	}
-	// parsing errors are handled above by AddPersistentPeers
-	_ = p2pPeers.DialPeersAsync(peers)
 	return &ctypes.ResultDialPeers{Log: "Dialing peers in progress. See /net_info for details"}, nil
 }