From 5b19fcf20450de8351213ea3dd8ada1f820b444d Mon Sep 17 00:00:00 2001
From: Jae Kwon <jae@tendermint.com>
Date: Sun, 11 Nov 2018 03:50:25 -0800
Subject: [PATCH] p2p: AddressBook requires addresses to have IDs; Do not close
 conn immediately after sending pex addrs in seed mode (#2797)

* Require addressbook to only store addresses with valid ID

* Do not shut down peer immediately after sending pex addrs in SeedMode

* p2p: fix #2773

* seed mode: use go-routine to sleep before stopping peer
---
 CHANGELOG.md           |  4 ++--
 p2p/netaddress.go      | 18 ++++++++++++++++--
 p2p/node_info.go       |  3 ++-
 p2p/pex/addrbook.go    |  4 ++++
 p2p/pex/errors.go      |  8 ++++++++
 p2p/pex/pex_reactor.go |  6 +++++-
 p2p/switch.go          |  2 +-
 7 files changed, 38 insertions(+), 7 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 792386e5..fd36e385 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -139,8 +139,8 @@ increasing attention to backwards compatibility. Thanks for bearing with us!
 - [node] [\#2434](https://github.com/tendermint/tendermint/issues/2434) Make node respond to signal interrupts while sleeping for genesis time
 - [state] [\#2616](https://github.com/tendermint/tendermint/issues/2616) Pass nil to NewValidatorSet() when genesis file's Validators field is nil
 - [p2p] [\#2555](https://github.com/tendermint/tendermint/issues/2555) Fix p2p switch FlushThrottle value (@goolAdapter)
-- [p2p] [\#2668](https://github.com/tendermint/tendermint/issues/2668) Reconnect to originally dialed address (not self-reported
-  address) for persistent peers
+- [p2p] [\#2668](https://github.com/tendermint/tendermint/issues/2668) Reconnect to originally dialed address (not self-reported address) for persistent peers
+- [p2p] [\#2797](https://github.com/tendermint/tendermint/pull/2797) AddressBook requires addresses to have IDs; Do not crap out immediately after sending pex addrs in seed mode
 
 
 ## v0.25.0
diff --git a/p2p/netaddress.go b/p2p/netaddress.go
index ec9a0ea7..f60271bc 100644
--- a/p2p/netaddress.go
+++ b/p2p/netaddress.go
@@ -32,8 +32,10 @@ type NetAddress struct {
 	str string
 }
 
-// IDAddressString returns id@hostPort.
-func IDAddressString(id ID, hostPort string) string {
+// IDAddressString returns id@hostPort. It strips the leading
+// protocol from protocolHostPort if it exists.
+func IDAddressString(id ID, protocolHostPort string) string {
+	hostPort := removeProtocolIfDefined(protocolHostPort)
 	return fmt.Sprintf("%s@%s", id, hostPort)
 }
 
@@ -218,10 +220,22 @@ func (na *NetAddress) Routable() bool {
 // For IPv4 these are either a 0 or all bits set address. For IPv6 a zero
 // address or one that matches the RFC3849 documentation address format.
 func (na *NetAddress) Valid() bool {
+	if string(na.ID) != "" {
+		data, err := hex.DecodeString(string(na.ID))
+		if err != nil || len(data) != IDByteLength {
+			return false
+		}
+	}
 	return na.IP != nil && !(na.IP.IsUnspecified() || na.RFC3849() ||
 		na.IP.Equal(net.IPv4bcast))
 }
 
+// HasID returns true if the address has an ID.
+// NOTE: It does not check whether the ID is valid or not.
+func (na *NetAddress) HasID() bool {
+	return string(na.ID) != ""
+}
+
 // Local returns true if it is a local address.
 func (na *NetAddress) Local() bool {
 	return na.IP.IsLoopback() || zero4.Contains(na.IP)
diff --git a/p2p/node_info.go b/p2p/node_info.go
index e46174e0..c36d98d9 100644
--- a/p2p/node_info.go
+++ b/p2p/node_info.go
@@ -216,7 +216,8 @@ OUTER_LOOP:
 // ListenAddr. Note that the ListenAddr is not authenticated and
 // may not match that address actually dialed if its an outbound peer.
 func (info DefaultNodeInfo) NetAddress() *NetAddress {
-	netAddr, err := NewNetAddressString(IDAddressString(info.ID(), info.ListenAddr))
+	idAddr := IDAddressString(info.ID(), info.ListenAddr)
+	netAddr, err := NewNetAddressString(idAddr)
 	if err != nil {
 		switch err.(type) {
 		case ErrNetAddressLookup:
diff --git a/p2p/pex/addrbook.go b/p2p/pex/addrbook.go
index 61710bbf..405a4628 100644
--- a/p2p/pex/addrbook.go
+++ b/p2p/pex/addrbook.go
@@ -652,6 +652,10 @@ func (a *addrBook) addAddress(addr, src *p2p.NetAddress) error {
 		return ErrAddrBookInvalidAddr{addr}
 	}
 
+	if !addr.HasID() {
+		return ErrAddrBookInvalidAddrNoID{addr}
+	}
+
 	// TODO: we should track ourAddrs by ID and by IP:PORT and refuse both.
 	if _, ok := a.ourAddrs[addr.String()]; ok {
 		return ErrAddrBookSelf{addr}
diff --git a/p2p/pex/errors.go b/p2p/pex/errors.go
index fbee748a..1f44ceee 100644
--- a/p2p/pex/errors.go
+++ b/p2p/pex/errors.go
@@ -54,3 +54,11 @@ type ErrAddrBookInvalidAddr struct {
 func (err ErrAddrBookInvalidAddr) Error() string {
 	return fmt.Sprintf("Cannot add invalid address %v", err.Addr)
 }
+
+type ErrAddrBookInvalidAddrNoID struct {
+	Addr *p2p.NetAddress
+}
+
+func (err ErrAddrBookInvalidAddrNoID) Error() string {
+	return fmt.Sprintf("Cannot add address with no ID %v", err.Addr)
+}
diff --git a/p2p/pex/pex_reactor.go b/p2p/pex/pex_reactor.go
index 46a12c48..85d292b0 100644
--- a/p2p/pex/pex_reactor.go
+++ b/p2p/pex/pex_reactor.go
@@ -221,7 +221,11 @@ func (r *PEXReactor) Receive(chID byte, src Peer, msgBytes []byte) {
 		// 2) limit the output size
 		if r.config.SeedMode {
 			r.SendAddrs(src, r.book.GetSelectionWithBias(biasToSelectNewPeers))
-			r.Switch.StopPeerGracefully(src)
+			go func() {
+				// TODO Fix properly #2092
+				time.Sleep(time.Second * 5)
+				r.Switch.StopPeerGracefully(src)
+			}()
 		} else {
 			r.SendAddrs(src, r.book.GetSelection())
 		}
diff --git a/p2p/switch.go b/p2p/switch.go
index b1406b9b..47a42cd0 100644
--- a/p2p/switch.go
+++ b/p2p/switch.go
@@ -616,7 +616,7 @@ func (sw *Switch) addPeer(p Peer) error {
 		return err
 	}
 
-	p.SetLogger(sw.Logger.With("peer", p.NodeInfo().NetAddress().String))
+	p.SetLogger(sw.Logger.With("peer", p.NodeInfo().NetAddress()))
 
 	// All good. Start peer
 	if sw.IsRunning() {