p2p: Restore OriginalAddr (#2668)

* p2p: bring back OriginalAddr

* p2p: set OriginalAddr

* update changelog

CHANGELOG_PENDING.md

@@ -82,3 +82,5 @@ Proposal or timeoutProposal before entering prevote
 - [p2p] \#2555 fix p2p switch FlushThrottle value (@goolAdapter)
 - [libs/event] \#2518 fix event concurrency flaw (@goolAdapter)
 - [state] \#2616 Pass nil to NewValidatorSet() when genesis file's Validators field is nil
+- [p2p] \#2668 Reconnect to originally dialed address (not self-reported
+  address) for persistent peers

blockchain/reactor_test.go

@@ -206,3 +206,4 @@ func (tp *bcrTestPeer) IsPersistent() bool                   { return true }
 func (tp *bcrTestPeer) Get(s string) interface{}             { return s }
 func (tp *bcrTestPeer) Set(string, interface{})              {}
 func (tp *bcrTestPeer) RemoteIP() net.IP                     { return []byte{127, 0, 0, 1} }
+func (tp *bcrTestPeer) OriginalAddr() *p2p.NetAddress        { return nil }

p2p/dummy/peer.go

@@ -78,3 +78,8 @@ func (p *peer) Get(key string) interface{} {
 	}
 	return nil
 }
+
+// OriginalAddr always returns nil.
+func (p *peer) OriginalAddr() *p2p.NetAddress {
+	return nil
+}

p2p/peer.go

@@ -26,6 +26,7 @@ type Peer interface {
 
 	NodeInfo() NodeInfo // peer's info
 	Status() tmconn.ConnectionStatus
+	OriginalAddr() *NetAddress
 
 	Send(byte, []byte) bool
 	TrySend(byte, []byte) bool
@@ -43,10 +44,28 @@ type peerConn struct {
 	config     *config.P2PConfig
 	conn       net.Conn // source connection
 
+	originalAddr *NetAddress // nil for inbound connections
+
 	// cached RemoteIP()
 	ip net.IP
 }
 
+func newPeerConn(
+	outbound, persistent bool,
+	config *config.P2PConfig,
+	conn net.Conn,
+	originalAddr *NetAddress,
+) peerConn {
+
+	return peerConn{
+		outbound:     outbound,
+		persistent:   persistent,
+		config:       config,
+		conn:         conn,
+		originalAddr: originalAddr,
+	}
+}
+
 // ID only exists for SecretConnection.
 // NOTE: Will panic if conn is not *SecretConnection.
 func (pc peerConn) ID() ID {
@@ -195,6 +214,15 @@ func (p *peer) NodeInfo() NodeInfo {
 	return p.nodeInfo
 }
 
+// OriginalAddr returns the original address, which was used to connect with
+// the peer. Returns nil for inbound peers.
+func (p *peer) OriginalAddr() *NetAddress {
+	if p.peerConn.outbound {
+		return p.peerConn.originalAddr
+	}
+	return nil
+}
+
 // Status returns the peer's ConnectionStatus.
 func (p *peer) Status() tmconn.ConnectionStatus {
 	return p.mconn.Status()

p2p/peer_set_test.go

@@ -28,6 +28,7 @@ func (mp *mockPeer) IsPersistent() bool                      { return true }
 func (mp *mockPeer) Get(s string) interface{}                { return s }
 func (mp *mockPeer) Set(string, interface{})                 {}
 func (mp *mockPeer) RemoteIP() net.IP                        { return mp.ip }
+func (mp *mockPeer) OriginalAddr() *NetAddress               { return nil }
 
 // Returns a mock peer
 func newMockPeer(ip net.IP) *mockPeer {

p2p/peer_test.go

@@ -114,7 +114,7 @@ func testOutboundPeerConn(
 		return peerConn{}, cmn.ErrorWrap(err, "Error creating peer")
 	}
 
-	pc, err := testPeerConn(conn, config, true, persistent, ourNodePrivKey)
+	pc, err := testPeerConn(conn, config, true, persistent, ourNodePrivKey, addr)
 	if err != nil {
 		if cerr := conn.Close(); cerr != nil {
 			return peerConn{}, cmn.ErrorWrap(err, cerr.Error())

p2p/pex/pex_reactor_test.go

@@ -402,6 +402,7 @@ func (mockPeer) Send(byte, []byte) bool        { return false }
 func (mockPeer) TrySend(byte, []byte) bool     { return false }
 func (mockPeer) Set(string, interface{})       {}
 func (mockPeer) Get(string) interface{}        { return nil }
+func (mockPeer) OriginalAddr() *p2p.NetAddress { return nil }
 
 func assertPeersWithTimeout(
 	t *testing.T,

p2p/switch.go

@@ -280,9 +280,12 @@ func (sw *Switch) StopPeerForError(peer Peer, reason interface{}) {
 	sw.stopAndRemovePeer(peer, reason)
 
 	if peer.IsPersistent() {
-		// TODO: use the original address dialed, not the self reported one
+		addr := peer.OriginalAddr()
-		// See #2618.
+		if addr == nil {
-		addr := peer.NodeInfo().NetAddress()
+			// FIXME: persistent peers can't be inbound right now.
+			// self-reported address for inbound persistent peers
+			addr = peer.NodeInfo().NetAddress()
+		}
 		go sw.reconnectToPeer(addr)
 	}
 }

p2p/test_util.go

@@ -206,7 +206,7 @@ func testInboundPeerConn(
 	config *config.P2PConfig,
 	ourNodePrivKey crypto.PrivKey,
 ) (peerConn, error) {
-	return testPeerConn(conn, config, false, false, ourNodePrivKey)
+	return testPeerConn(conn, config, false, false, ourNodePrivKey, nil)
 }
 
 func testPeerConn(
@@ -214,6 +214,7 @@ func testPeerConn(
 	cfg *config.P2PConfig,
 	outbound, persistent bool,
 	ourNodePrivKey crypto.PrivKey,
+	originalAddr *NetAddress,
 ) (pc peerConn, err error) {
 	conn := rawConn
 
@@ -231,10 +232,11 @@ func testPeerConn(
 
 	// Only the information we already have
 	return peerConn{
-		config:     cfg,
+		config:       cfg,
-		outbound:   outbound,
+		outbound:     outbound,
-		persistent: persistent,
+		persistent:   persistent,
-		conn:       conn,
+		conn:         conn,
+		originalAddr: originalAddr,
 	}, nil
 }
 

p2p/transport.go

@@ -171,7 +171,7 @@ func (mt *MultiplexTransport) Accept(cfg peerConfig) (Peer, error) {
 
 		cfg.outbound = false
 
-		return mt.wrapPeer(a.conn, a.nodeInfo, cfg), nil
+		return mt.wrapPeer(a.conn, a.nodeInfo, cfg, nil), nil
 	case <-mt.closec:
 		return nil, &ErrTransportClosed{}
 	}
@@ -199,7 +199,7 @@ func (mt *MultiplexTransport) Dial(
 
 	cfg.outbound = true
 
-	p := mt.wrapPeer(secretConn, nodeInfo, cfg)
+	p := mt.wrapPeer(secretConn, nodeInfo, cfg, &addr)
 
 	return p, nil
 }
@@ -399,14 +399,19 @@ func (mt *MultiplexTransport) wrapPeer(
 	c net.Conn,
 	ni NodeInfo,
 	cfg peerConfig,
+	dialedAddr *NetAddress,
 ) Peer {
+
+	peerConn := newPeerConn(
+		cfg.outbound,
+		cfg.persistent,
+		&mt.p2pConfig,
+		c,
+		dialedAddr,
+	)
+
 	p := newPeer(
-		peerConn{
+		peerConn,
-			conn:       c,
-			config:     &mt.p2pConfig,
-			outbound:   cfg.outbound,
-			persistent: cfg.persistent,
-		},
 		mt.mConfig,
 		ni,
 		cfg.reactorsByCh,