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

@@ -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

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)

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:

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}

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)
+}

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())
 		}

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() {