diff --git a/p2p/dummy/peer.go b/p2p/dummy/peer.go index 97fb7e2ef..fc2242366 100644 --- a/p2p/dummy/peer.go +++ b/p2p/dummy/peer.go @@ -1,6 +1,8 @@ package dummy import ( + "net" + p2p "github.com/tendermint/tendermint/p2p" tmconn "github.com/tendermint/tendermint/p2p/conn" cmn "github.com/tendermint/tmlibs/common" @@ -19,6 +21,7 @@ func NewPeer() *peer { kv: make(map[string]interface{}), } p.BaseService = *cmn.NewBaseService(nil, "peer", p) + return p } @@ -42,6 +45,11 @@ func (p *peer) NodeInfo() p2p.NodeInfo { return p2p.NodeInfo{} } +// RemoteIP always returns localhost. +func (p *peer) RemoteIP() net.IP { + return net.ParseIP("127.0.0.1") +} + // Status always returns empry connection status. func (p *peer) Status() tmconn.ConnectionStatus { return tmconn.ConnectionStatus{} diff --git a/p2p/errors.go b/p2p/errors.go index 314507785..fc477d1c2 100644 --- a/p2p/errors.go +++ b/p2p/errors.go @@ -2,30 +2,36 @@ package p2p import ( "fmt" + "net" ) +// ErrSwitchDuplicatePeerID to be raised when a peer is connecting with a known +// ID. type ErrSwitchDuplicatePeerID struct { ID ID } func (e ErrSwitchDuplicatePeerID) Error() string { - return fmt.Errorf("Duplicate peer ID %v", e.ID) + return fmt.Sprintf("Duplicate peer ID %v", e.ID) } +// ErrSwitchDuplicatePeerIP to be raised whena a peer is connecting with a known +// IP. type ErrSwitchDuplicatePeerIP struct { - Addr string + IP net.IP } func (e ErrSwitchDuplicatePeerIP) Error() string { - return fmt.Errorf("Duplicate peer IP %v", e.Addr) + return fmt.Sprintf("Duplicate peer IP %v", e.IP.String()) } +// ErrSwitchConnectToSelf to be raised when trying to connect to itself. type ErrSwitchConnectToSelf struct { Addr *NetAddress } func (e ErrSwitchConnectToSelf) Error() string { - return fmt.Errorf("Connect to self: %v", e.Addr) + return fmt.Sprintf("Connect to self: %v", e.Addr) } type ErrSwitchAuthenticationFailure struct { @@ -34,7 +40,11 @@ type ErrSwitchAuthenticationFailure struct { } func (e ErrSwitchAuthenticationFailure) Error() string { - return fmt.Sprintf("Failed to authenticate peer. Dialed %v, but got peer with ID %s", e.Dialed, e.Got) + return fmt.Sprintf( + "Failed to authenticate peer. Dialed %v, but got peer with ID %s", + e.Dialed, + e.Got, + ) } //------------------------------------------------------------------- diff --git a/p2p/peer.go b/p2p/peer.go index 1b5ec0ae4..2cb3d9f9f 100644 --- a/p2p/peer.go +++ b/p2p/peer.go @@ -17,7 +17,7 @@ type Peer interface { cmn.Service ID() ID // peer's cryptographic ID - RemoteIP() string // remote IP of the connection + RemoteIP() net.IP // remote IP of the connection IsOutbound() bool // did we dial the peer IsPersistent() bool // do we redial this peer when we disconnect NodeInfo() NodeInfo // peer's info @@ -38,6 +38,7 @@ type peerConn struct { persistent bool config *PeerConfig conn net.Conn // source connection + ips []net.IP } // ID only exists for SecretConnection. @@ -47,12 +48,24 @@ func (pc peerConn) ID() ID { } // Return the IP from the connection RemoteAddr -func (pc peerConn) RemoteIP() string { +func (pc peerConn) RemoteIP() net.IP { + if len(pc.ips) > 0 { + return pc.ips[0] + } + host, _, err := net.SplitHostPort(pc.conn.RemoteAddr().String()) if err != nil { panic(err) } - return host + + ips, err := net.LookupIP(host) + if err != nil { + panic(err) + } + + pc.ips = ips + + return ips[0] } // peer implements Peer. diff --git a/p2p/peer_set.go b/p2p/peer_set.go index 3ac48ea5e..a46d18b17 100644 --- a/p2p/peer_set.go +++ b/p2p/peer_set.go @@ -1,13 +1,14 @@ package p2p import ( + "net" "sync" ) // IPeerSet has a (immutable) subset of the methods of PeerSet. type IPeerSet interface { Has(key ID) bool - HasIP(ip string) bool + HasIP(ip net.IP) bool Get(key ID) Peer List() []Peer Size() int @@ -18,10 +19,9 @@ type IPeerSet interface { // PeerSet is a special structure for keeping a table of peers. // Iteration over the peers is super fast and thread-safe. type PeerSet struct { - mtx sync.Mutex - lookup map[ID]*peerSetItem - lookupIP map[string]struct{} - list []Peer + mtx sync.Mutex + lookup map[ID]*peerSetItem + list []Peer } type peerSetItem struct { @@ -32,21 +32,21 @@ type peerSetItem struct { // NewPeerSet creates a new peerSet with a list of initial capacity of 256 items. func NewPeerSet() *PeerSet { return &PeerSet{ - lookup: make(map[ID]*peerSetItem), - lookupIP: make(map[string]struct{}), - list: make([]Peer, 0, 256), + lookup: make(map[ID]*peerSetItem), + list: make([]Peer, 0, 256), } } // Add adds the peer to the PeerSet. -// It returns ErrSwitchDuplicatePeer if the peer is already present. +// It returns an error carrying the reason, if the peer is already present. func (ps *PeerSet) Add(peer Peer) error { ps.mtx.Lock() defer ps.mtx.Unlock() if ps.lookup[peer.ID()] != nil { return ErrSwitchDuplicatePeerID{peer.ID()} } - if _, ok := ps.lookupIP[peer.RemoteIP()]; ok { + + if ps.HasIP(peer.RemoteIP()) { return ErrSwitchDuplicatePeerIP{peer.RemoteIP()} } @@ -55,7 +55,6 @@ func (ps *PeerSet) Add(peer Peer) error { // iterating over the ps.list slice. ps.list = append(ps.list, peer) ps.lookup[peer.ID()] = &peerSetItem{peer, index} - ps.lookupIP[peer.RemoteIP()] = struct{}{} return nil } @@ -68,13 +67,19 @@ func (ps *PeerSet) Has(peerKey ID) bool { return ok } -// HasIP returns true iff the PeerSet contains -// the peer referred to by this IP address. -func (ps *PeerSet) HasIP(peerIP string) bool { +// HasIP returns true if the PeerSet contains the peer referred to by this IP +// address. +func (ps *PeerSet) HasIP(peerIP net.IP) bool { ps.mtx.Lock() - _, ok := ps.lookupIP[peerIP] ps.mtx.Unlock() - return ok + + for _, item := range ps.lookup { + if item.peer.RemoteIP().Equal(peerIP) { + return true + } + } + + return false } // Get looks up a peer by the provided peerKey. @@ -92,7 +97,7 @@ func (ps *PeerSet) Get(peerKey ID) Peer { func (ps *PeerSet) Remove(peer Peer) { ps.mtx.Lock() defer ps.mtx.Unlock() - delete(ps.lookupIP[peer.RemoteIP()]) + item := ps.lookup[peer.ID()] if item == nil { return diff --git a/p2p/peer_set_test.go b/p2p/peer_set_test.go index 872758355..fafe1e262 100644 --- a/p2p/peer_set_test.go +++ b/p2p/peer_set_test.go @@ -112,18 +112,24 @@ func TestPeerSetAddDuplicate(t *testing.T) { } // Now collect and tally the results - errsTally := make(map[error]int) + errsTally := make(map[string]int) for i := 0; i < n; i++ { err := <-errsChan - errsTally[err]++ + + switch err.(type) { + case ErrSwitchDuplicatePeerID: + errsTally["duplicateID"]++ + default: + errsTally["other"]++ + } } // Our next procedure is to ensure that only one addition // succeeded and that the rest are each ErrSwitchDuplicatePeer. - wantErrCount, gotErrCount := n-1, errsTally[ErrSwitchDuplicatePeer] + wantErrCount, gotErrCount := n-1, errsTally["duplicateID"] assert.Equal(t, wantErrCount, gotErrCount, "invalid ErrSwitchDuplicatePeer count") - wantNilErrCount, gotNilErrCount := 1, errsTally[nil] + wantNilErrCount, gotNilErrCount := 1, errsTally["other"] assert.Equal(t, wantNilErrCount, gotNilErrCount, "invalid nil errCount") } diff --git a/p2p/pex/pex_reactor_test.go b/p2p/pex/pex_reactor_test.go index f7297a343..38aa84059 100644 --- a/p2p/pex/pex_reactor_test.go +++ b/p2p/pex/pex_reactor_test.go @@ -3,6 +3,7 @@ package pex import ( "fmt" "io/ioutil" + "net" "os" "path/filepath" "testing" @@ -365,6 +366,7 @@ func (mp mockPeer) NodeInfo() p2p.NodeInfo { ListenAddr: mp.addr.DialString(), } } +func (mp mockPeer) RemoteIP() net.IP { return net.ParseIP("127.0.0.1") } func (mp mockPeer) Status() conn.ConnectionStatus { return conn.ConnectionStatus{} } func (mp mockPeer) Send(byte, []byte) bool { return false } func (mp mockPeer) TrySend(byte, []byte) bool { return false } diff --git a/p2p/switch.go b/p2p/switch.go index 61c9ce969..22307bd91 100644 --- a/p2p/switch.go +++ b/p2p/switch.go @@ -404,7 +404,7 @@ func (sw *Switch) DialPeersAsync(addrBook AddrBook, peers []string, persistent b err := sw.DialPeerWithAddress(addr, persistent) if err != nil { switch err.(type) { - case ErrSwitchConnectToSelf, ErrSwitchDuplicatePeer: + case ErrSwitchConnectToSelf, ErrSwitchDuplicatePeerID: sw.Logger.Debug("Error dialing peer", "err", err) default: sw.Logger.Error("Error dialing peer", "err", err) @@ -579,11 +579,8 @@ func (sw *Switch) addPeer(pc peerConn) error { } // check ips for both the connection addr and the self reported addr - if sw.peers.HasIP(addr) { - return ErrSwitchDuplicatePeerIP{addr} - } - if sw.peers.HasIP(peerNodeInfo.ListenAddr) { - return ErrSwitchDuplicatePeerIP{peerNodeInfo.ListenAddr} + if sw.peers.HasIP(pc.RemoteIP()) { + return ErrSwitchDuplicatePeerIP{pc.RemoteIP()} } // Filter peer against ID white list diff --git a/p2p/switch_test.go b/p2p/switch_test.go index 25ed73bce..373378d73 100644 --- a/p2p/switch_test.go +++ b/p2p/switch_test.go @@ -193,7 +193,7 @@ func TestSwitchFiltersOutItself(t *testing.T) { // addr should be rejected in addPeer based on the same ID err := s1.DialPeerWithAddress(rp.Addr(), false) if assert.Error(t, err) { - assert.Equal(t, ErrSwitchConnectToSelf, err) + assert.EqualValues(t, ErrSwitchConnectToSelf{}, err) } assert.True(t, s1.addrBook.OurAddress(rp.Addr()))