some fixes from review

p2p/addrbook_test.go

@@ -190,7 +190,7 @@ func randIPv4Address(t *testing.T) *NetAddress {
 		)
 		port := rand.Intn(65535-1) + 1
 		addr, err := NewNetAddressString(fmt.Sprintf("%v:%v", ip, port))
-		addr.ID = ID(hex.EncodeToString(cmn.RandBytes(20))) // TODO
+		addr.ID = ID(hex.EncodeToString(cmn.RandBytes(20)))
 		assert.Nil(t, err, "error generating rand network address")
 		if addr.Routable() {
 			return addr

p2p/key.go

@@ -11,8 +11,13 @@ import (
 	cmn "github.com/tendermint/tmlibs/common"
 )
 
+// ID is a hex-encoded crypto.Address
 type ID string
 
+// IDByteLength is the length of a crypto.Address. Currently only 20.
+// TODO: support other length addresses ?
+const IDByteLength = 20
+
 //------------------------------------------------------------------------------
 // Persistent peer ID
 // TODO: encrypt on disk

p2p/netaddress.go

@@ -50,8 +50,8 @@ func NewNetAddress(id ID, addr net.Addr) *NetAddress {
 }
 
 // NewNetAddressString returns a new NetAddress using the provided
-// address in the form of "IP:Port". Also resolves the host if host
-// is not an IP.
+// address in the form of "ID@IP:Port", where the ID is optional.
+// Also resolves the host if host is not an IP.
 func NewNetAddressString(addr string) (*NetAddress, error) {
 	addr = removeProtocolIfDefined(addr)
 
@@ -63,8 +63,9 @@ func NewNetAddressString(addr string) (*NetAddress, error) {
 		if err != nil {
 			return nil, errors.Wrap(err, fmt.Sprintf("Address (%s) contains invalid ID", addr))
 		}
-		if len(idBytes) != 20 {
-			return nil, fmt.Errorf("Address (%s) contains ID of invalid length (%d). Should be 20 hex-encoded bytes", len(idBytes))
+		if len(idBytes) != IDByteLength {
+			return nil, fmt.Errorf("Address (%s) contains ID of invalid length (%d). Should be %d hex-encoded bytes",
+				addr, len(idBytes), IDByteLength)
 		}
 		id, addr = ID(idStr), spl[1]
 	}

p2p/netaddress_test.go

@@ -48,6 +48,13 @@ func TestNewNetAddressString(t *testing.T) {
 		{"tcp://this-isnot-hex@127.0.0.1:8080", "", false},
 		{"tcp://xxxxbeefdeadbeefdeadbeefdeadbeefdeadbeef@127.0.0.1:8080", "", false},
 		{"tcp://deadbeefdeadbeefdeadbeefdeadbeefdeadbeef@127.0.0.1:8080", "deadbeefdeadbeefdeadbeefdeadbeefdeadbeef@127.0.0.1:8080", true},
+
+		{"tcp://@127.0.0.1:8080", "", false},
+		{"tcp://@", "", false},
+		{"", "", false},
+		{"@", "", false},
+		{" @", "", false},
+		{" @ ", "", false},
 	}
 
 	for _, tc := range testCases {

p2p/pex_reactor.go

@@ -109,19 +109,20 @@ func (r *PEXReactor) GetChannels() []*ChannelDescriptor {
 func (r *PEXReactor) AddPeer(p Peer) {
 	if p.IsOutbound() {
 		// For outbound peers, the address is already in the books.
-		// Either it was added in DialPersistentPeers or when we
+		// Either it was added in DialPeersAsync or when we
 		// received the peer's address in r.Receive
 		if r.book.NeedMoreAddrs() {
 			r.RequestPEX(p)
 		}
-	} else { // For inbound connections, the peer is its own source
-		addr, err := NewNetAddressString(p.NodeInfo().ListenAddr)
-		addr.ID = p.ID() // TODO: handle in NewNetAddress func
+	} else {
+		addrStr := fmt.Sprintf("%s@%s", p.ID(), p.NodeInfo().ListenAddr)
+		addr, err := NewNetAddressString(addrStr)
 		if err != nil {
 			// peer gave us a bad ListenAddr. TODO: punish
 			r.Logger.Error("Error in AddPeer: invalid peer address", "addr", p.NodeInfo().ListenAddr, "err", err)
 			return
 		}
+		// For inbound connections, the peer is its own source
 		r.book.AddAddress(addr, addr)
 	}
 }

p2p/switch.go

@@ -94,6 +94,7 @@ type Switch struct {
 
 var (
 	ErrSwitchDuplicatePeer = errors.New("Duplicate peer")
+	ErrSwitchConnectToSelf = errors.New("Connect to self")
 )
 
 func NewSwitch(config *cfg.P2PConfig) *Switch {
@@ -241,7 +242,7 @@ func (sw *Switch) OnStop() {
 func (sw *Switch) addPeer(peer *peer) error {
 	// Avoid self
 	if sw.nodeKey.ID() == peer.ID() {
-		return errors.New("Ignoring connection from self")
+		return ErrSwitchConnectToSelf
 	}
 
 	// Filter peer against white list