From 1fe41be9294662d08641ae2f7048c689a9f800cb Mon Sep 17 00:00:00 2001 From: Ethan Buchman Date: Sat, 28 Apr 2018 16:45:08 -0400 Subject: [PATCH 01/14] p2p: prevent connections from same ip --- p2p/errors.go | 28 +++++++++++++++++++++++----- p2p/peer.go | 10 ++++++++++ p2p/peer_set.go | 29 +++++++++++++++++++++++------ p2p/switch.go | 24 +++++++++++++++--------- 4 files changed, 71 insertions(+), 20 deletions(-) diff --git a/p2p/errors.go b/p2p/errors.go index f4a09e6c0..314507785 100644 --- a/p2p/errors.go +++ b/p2p/errors.go @@ -1,14 +1,32 @@ package p2p import ( - "errors" "fmt" ) -var ( - ErrSwitchDuplicatePeer = errors.New("Duplicate peer") - ErrSwitchConnectToSelf = errors.New("Connect to self") -) +type ErrSwitchDuplicatePeerID struct { + ID ID +} + +func (e ErrSwitchDuplicatePeerID) Error() string { + return fmt.Errorf("Duplicate peer ID %v", e.ID) +} + +type ErrSwitchDuplicatePeerIP struct { + Addr string +} + +func (e ErrSwitchDuplicatePeerIP) Error() string { + return fmt.Errorf("Duplicate peer IP %v", e.Addr) +} + +type ErrSwitchConnectToSelf struct { + Addr *NetAddress +} + +func (e ErrSwitchConnectToSelf) Error() string { + return fmt.Errorf("Connect to self: %v", e.Addr) +} type ErrSwitchAuthenticationFailure struct { Dialed *NetAddress diff --git a/p2p/peer.go b/p2p/peer.go index b9c8f8b41..1b5ec0ae4 100644 --- a/p2p/peer.go +++ b/p2p/peer.go @@ -17,6 +17,7 @@ type Peer interface { cmn.Service ID() ID // peer's cryptographic ID + RemoteIP() string // 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 @@ -45,6 +46,15 @@ func (pc peerConn) ID() ID { return PubKeyToID(pc.conn.(*tmconn.SecretConnection).RemotePubKey()) } +// Return the IP from the connection RemoteAddr +func (pc peerConn) RemoteIP() string { + host, _, err := net.SplitHostPort(pc.conn.RemoteAddr().String()) + if err != nil { + panic(err) + } + return host +} + // peer implements Peer. // // Before using a peer, you will need to perform a handshake on connection. diff --git a/p2p/peer_set.go b/p2p/peer_set.go index a4565ea1d..3ac48ea5e 100644 --- a/p2p/peer_set.go +++ b/p2p/peer_set.go @@ -7,6 +7,7 @@ import ( // IPeerSet has a (immutable) subset of the methods of PeerSet. type IPeerSet interface { Has(key ID) bool + HasIP(ip string) bool Get(key ID) Peer List() []Peer Size() int @@ -17,9 +18,10 @@ 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 - list []Peer + mtx sync.Mutex + lookup map[ID]*peerSetItem + lookupIP map[string]struct{} + list []Peer } type peerSetItem struct { @@ -30,8 +32,9 @@ 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), - list: make([]Peer, 0, 256), + lookup: make(map[ID]*peerSetItem), + lookupIP: make(map[string]struct{}), + list: make([]Peer, 0, 256), } } @@ -41,7 +44,10 @@ func (ps *PeerSet) Add(peer Peer) error { ps.mtx.Lock() defer ps.mtx.Unlock() if ps.lookup[peer.ID()] != nil { - return ErrSwitchDuplicatePeer + return ErrSwitchDuplicatePeerID{peer.ID()} + } + if _, ok := ps.lookupIP[peer.RemoteIP()]; ok { + return ErrSwitchDuplicatePeerIP{peer.RemoteIP()} } index := len(ps.list) @@ -49,6 +55,7 @@ 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 } @@ -61,6 +68,15 @@ 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 { + ps.mtx.Lock() + _, ok := ps.lookupIP[peerIP] + ps.mtx.Unlock() + return ok +} + // Get looks up a peer by the provided peerKey. func (ps *PeerSet) Get(peerKey ID) Peer { ps.mtx.Lock() @@ -76,6 +92,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/switch.go b/p2p/switch.go index f62e5f992..61c9ce969 100644 --- a/p2p/switch.go +++ b/p2p/switch.go @@ -403,7 +403,7 @@ func (sw *Switch) DialPeersAsync(addrBook AddrBook, peers []string, persistent b sw.randomSleep(0) err := sw.DialPeerWithAddress(addr, persistent) if err != nil { - switch err { + switch err.(type) { case ErrSwitchConnectToSelf, ErrSwitchDuplicatePeer: sw.Logger.Debug("Error dialing peer", "err", err) default: @@ -534,6 +534,8 @@ func (sw *Switch) addPeer(pc peerConn) error { return err } + // dont connect to multiple peers on the same IP + // NOTE: if AuthEnc==false, we don't have a peerID until after the handshake. // If AuthEnc==true then we already know the ID and could do the checks first before the handshake, // but it's simple to just deal with both cases the same after the handshake. @@ -564,20 +566,24 @@ func (sw *Switch) addPeer(pc peerConn) error { // Avoid self if sw.nodeKey.ID() == peerID { addr := peerNodeInfo.NetAddress() - - // remove the given address from the address book if we added it earlier + // remove the given address from the address book + // and add to our addresses to avoid dialing again sw.addrBook.RemoveAddress(addr) - - // add the given address to the address book to avoid dialing ourselves - // again this is our public address sw.addrBook.AddOurAddress(addr) - - return ErrSwitchConnectToSelf + return ErrSwitchConnectToSelf{addr} } // Avoid duplicate if sw.peers.Has(peerID) { - return ErrSwitchDuplicatePeer + return ErrSwitchDuplicatePeerID{peerID} + } + + // 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} } // Filter peer against ID white list From 77f09f5b5e898694b16ae9744466e843a5b77df4 Mon Sep 17 00:00:00 2001 From: Alexander Simmerl Date: Mon, 7 May 2018 17:15:12 +0200 Subject: [PATCH 02/14] Move to ne.IP --- p2p/dummy/peer.go | 8 ++++++++ p2p/errors.go | 20 ++++++++++++++----- p2p/peer.go | 19 +++++++++++++++--- p2p/peer_set.go | 39 +++++++++++++++++++++---------------- p2p/peer_set_test.go | 14 +++++++++---- p2p/pex/pex_reactor_test.go | 2 ++ p2p/switch.go | 9 +++------ p2p/switch_test.go | 2 +- 8 files changed, 77 insertions(+), 36 deletions(-) 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())) From c5f45275ec972936ac7f448f0436f08c727b320a Mon Sep 17 00:00:00 2001 From: Alexander Simmerl Date: Mon, 7 May 2018 17:54:55 +0200 Subject: [PATCH 03/14] Use remotePeer for test switch --- p2p/peer_test.go | 2 +- p2p/test_util.go | 34 +++++++++++++++++++++++++++++++++- 2 files changed, 34 insertions(+), 2 deletions(-) diff --git a/p2p/peer_test.go b/p2p/peer_test.go index 24d750a9f..0e21dcaf2 100644 --- a/p2p/peer_test.go +++ b/p2p/peer_test.go @@ -154,7 +154,7 @@ func (p *remotePeer) accept(l net.Listener) { _, err = pc.HandshakeTimeout(NodeInfo{ ID: p.Addr().ID, Moniker: "remote_peer", - Network: "testing", + Network: "localhost", Version: "123.123.123", ListenAddr: l.Addr().String(), Channels: []byte{testCh}, diff --git a/p2p/test_util.go b/p2p/test_util.go index 2c90bf516..e48e426dd 100644 --- a/p2p/test_util.go +++ b/p2p/test_util.go @@ -1,7 +1,9 @@ package p2p import ( + "fmt" "net" + "time" crypto "github.com/tendermint/go-crypto" cmn "github.com/tendermint/tmlibs/common" @@ -80,7 +82,37 @@ func MakeConnectedSwitches(cfg *cfg.P2PConfig, n int, initSwitch func(int, *Swit func Connect2Switches(switches []*Switch, i, j int) { switchI := switches[i] switchJ := switches[j] - c1, c2 := conn.NetPipe() + + p1 := &remotePeer{ + Config: switchJ.peerConfig, + PrivKey: switchJ.nodeKey.PrivKey, + } + p1.Start() + + c1, err := net.DialTimeout( + "tcp", + fmt.Sprintf("%s:%d", p1.addr.IP.String(), p1.addr.Port), + 100*time.Millisecond, + ) + if err != nil { + panic(err) + } + + p2 := &remotePeer{ + Config: switchI.peerConfig, + PrivKey: switchI.nodeKey.PrivKey, + } + p2.Start() + + c2, err := net.DialTimeout( + "tcp", + fmt.Sprintf("%s:%d", p2.addr.IP.String(), p2.addr.Port), + 100*time.Millisecond, + ) + if err != nil { + panic(err) + } + doneCh := make(chan struct{}) go func() { err := switchI.addPeerWithConnection(c1) From b698a9febcd41795bd94a824178c42b1742d0855 Mon Sep 17 00:00:00 2001 From: Alexander Simmerl Date: Wed, 16 May 2018 19:13:45 +0200 Subject: [PATCH 04/14] Remove double locking in HasIP --- consensus/byzantine_test.go | 2 +- p2p/peer_set.go | 11 ++++- p2p/peer_set_test.go | 37 ++++++++++++---- p2p/peer_test.go | 72 ------------------------------ p2p/test_util.go | 87 +++++++++++++++++++++++++++++++++++-- 5 files changed, 121 insertions(+), 88 deletions(-) diff --git a/consensus/byzantine_test.go b/consensus/byzantine_test.go index 5f04a3308..f18f16230 100644 --- a/consensus/byzantine_test.go +++ b/consensus/byzantine_test.go @@ -27,7 +27,7 @@ func init() { // Heal partition and ensure A sees the commit func TestByzantine(t *testing.T) { N := 4 - logger := consensusLogger() + logger := consensusLogger().With("test", "byzantine") css := randConsensusNet(N, "consensus_byzantine_test", newMockTickerFunc(false), newCounter) // give the byzantine validator a normal ticker diff --git a/p2p/peer_set.go b/p2p/peer_set.go index a46d18b17..66a7fdadb 100644 --- a/p2p/peer_set.go +++ b/p2p/peer_set.go @@ -42,11 +42,12 @@ func NewPeerSet() *PeerSet { 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 ps.HasIP(peer.RemoteIP()) { + if ps.hasIP(peer.RemoteIP()) { return ErrSwitchDuplicatePeerIP{peer.RemoteIP()} } @@ -71,8 +72,14 @@ func (ps *PeerSet) Has(peerKey ID) bool { // address. func (ps *PeerSet) HasIP(peerIP net.IP) bool { ps.mtx.Lock() - ps.mtx.Unlock() + defer ps.mtx.Unlock() + + return ps.hasIP(peerIP) +} +// hasIP does not acquire a lock so it can be used in public methods which +// already lock. +func (ps *PeerSet) hasIP(peerIP net.IP) bool { for _, item := range ps.lookup { if item.peer.RemoteIP().Equal(peerIP) { return true diff --git a/p2p/peer_set_test.go b/p2p/peer_set_test.go index fafe1e262..ff2bdbad5 100644 --- a/p2p/peer_set_test.go +++ b/p2p/peer_set_test.go @@ -2,6 +2,7 @@ package p2p import ( "math/rand" + "net" "sync" "testing" @@ -12,23 +13,34 @@ import ( ) // Returns an empty kvstore peer -func randPeer() *peer { +func randPeer(ip net.IP) *peer { + if ip == nil { + ip = net.IP{127, 0, 0, 1} + } + nodeKey := NodeKey{PrivKey: crypto.GenPrivKeyEd25519()} - return &peer{ + p := &peer{ nodeInfo: NodeInfo{ ID: nodeKey.ID(), ListenAddr: cmn.Fmt("%v.%v.%v.%v:46656", rand.Int()%256, rand.Int()%256, rand.Int()%256, rand.Int()%256), }, } + + p.ips = []net.IP{ + ip, + } + + return p } func TestPeerSetAddRemoveOne(t *testing.T) { t.Parallel() + peerSet := NewPeerSet() var peerList []Peer for i := 0; i < 5; i++ { - p := randPeer() + p := randPeer(net.IP{127, 0, 0, byte(i)}) if err := peerSet.Add(p); err != nil { t.Error(err) } @@ -72,7 +84,7 @@ func TestPeerSetAddRemoveMany(t *testing.T) { peers := []Peer{} N := 100 for i := 0; i < N; i++ { - peer := randPeer() + peer := randPeer(net.IP{127, 0, 0, byte(i)}) if err := peerSet.Add(peer); err != nil { t.Errorf("Failed to add new peer") } @@ -96,7 +108,7 @@ func TestPeerSetAddRemoveMany(t *testing.T) { func TestPeerSetAddDuplicate(t *testing.T) { t.Parallel() peerSet := NewPeerSet() - peer := randPeer() + peer := randPeer(nil) n := 20 errsChan := make(chan error) @@ -133,10 +145,17 @@ func TestPeerSetAddDuplicate(t *testing.T) { assert.Equal(t, wantNilErrCount, gotNilErrCount, "invalid nil errCount") } +func TestPeerSetAddDuplicateIP(t *testing.T) { +} + func TestPeerSetGet(t *testing.T) { t.Parallel() - peerSet := NewPeerSet() - peer := randPeer() + + var ( + peerSet = NewPeerSet() + peer = randPeer(nil) + ) + assert.Nil(t, peerSet.Get(peer.ID()), "expecting a nil lookup, before .Add") if err := peerSet.Add(peer); err != nil { @@ -150,8 +169,8 @@ func TestPeerSetGet(t *testing.T) { wg.Add(1) go func(i int) { defer wg.Done() - got, want := peerSet.Get(peer.ID()), peer - assert.Equal(t, got, want, "#%d: got=%v want=%v", i, got, want) + have, want := peerSet.Get(peer.ID()), peer + assert.Equal(t, have, want, "%d: have %v, want %v", i, have, want) }(i) } wg.Wait() diff --git a/p2p/peer_test.go b/p2p/peer_test.go index 0e21dcaf2..a93b2ceea 100644 --- a/p2p/peer_test.go +++ b/p2p/peer_test.go @@ -1,8 +1,6 @@ package p2p import ( - golog "log" - "net" "testing" "time" @@ -14,8 +12,6 @@ import ( "github.com/tendermint/tmlibs/log" ) -const testCh = 0x01 - func TestPeerBasic(t *testing.T) { assert, require := assert.New(t), require.New(t) @@ -109,71 +105,3 @@ func createOutboundPeerAndPerformHandshake(addr *NetAddress, config *PeerConfig) p.SetLogger(log.TestingLogger().With("peer", addr)) return p, nil } - -type remotePeer struct { - PrivKey crypto.PrivKey - Config *PeerConfig - addr *NetAddress - quit chan struct{} -} - -func (p *remotePeer) Addr() *NetAddress { - return p.addr -} - -func (p *remotePeer) ID() ID { - return PubKeyToID(p.PrivKey.PubKey()) -} - -func (p *remotePeer) Start() { - l, e := net.Listen("tcp", "127.0.0.1:0") // any available address - if e != nil { - golog.Fatalf("net.Listen tcp :0: %+v", e) - } - p.addr = NewNetAddress(PubKeyToID(p.PrivKey.PubKey()), l.Addr()) - p.quit = make(chan struct{}) - go p.accept(l) -} - -func (p *remotePeer) Stop() { - close(p.quit) -} - -func (p *remotePeer) accept(l net.Listener) { - conns := []net.Conn{} - - for { - conn, err := l.Accept() - if err != nil { - golog.Fatalf("Failed to accept conn: %+v", err) - } - pc, err := newInboundPeerConn(conn, p.Config, p.PrivKey) - if err != nil { - golog.Fatalf("Failed to create a peer: %+v", err) - } - _, err = pc.HandshakeTimeout(NodeInfo{ - ID: p.Addr().ID, - Moniker: "remote_peer", - Network: "localhost", - Version: "123.123.123", - ListenAddr: l.Addr().String(), - Channels: []byte{testCh}, - }, 1*time.Second) - if err != nil { - golog.Fatalf("Failed to perform handshake: %+v", err) - } - - conns = append(conns, conn) - - select { - case <-p.quit: - for _, conn := range conns { - if err := conn.Close(); err != nil { - golog.Fatal(err) - } - } - return - default: - } - } -} diff --git a/p2p/test_util.go b/p2p/test_util.go index e48e426dd..de9243499 100644 --- a/p2p/test_util.go +++ b/p2p/test_util.go @@ -2,6 +2,7 @@ package p2p import ( "fmt" + golog "log" "net" "time" @@ -13,6 +14,8 @@ import ( "github.com/tendermint/tendermint/p2p/conn" ) +const testCh = 0x01 + func AddPeerToSwitch(sw *Switch, peer Peer) { sw.peers.Add(peer) } @@ -84,8 +87,9 @@ func Connect2Switches(switches []*Switch, i, j int) { switchJ := switches[j] p1 := &remotePeer{ - Config: switchJ.peerConfig, - PrivKey: switchJ.nodeKey.PrivKey, + Config: switchJ.peerConfig, + PrivKey: switchJ.nodeKey.PrivKey, + channels: switchJ.NodeInfo().Channels, } p1.Start() @@ -99,8 +103,9 @@ func Connect2Switches(switches []*Switch, i, j int) { } p2 := &remotePeer{ - Config: switchI.peerConfig, - PrivKey: switchI.nodeKey.PrivKey, + Config: switchI.peerConfig, + PrivKey: switchI.nodeKey.PrivKey, + channels: switchI.NodeInfo().Channels, } p2.Start() @@ -183,3 +188,77 @@ func MakeSwitch(cfg *cfg.P2PConfig, i int, network, version string, initSwitch f sw.SetNodeKey(nodeKey) return sw } + +type remotePeer struct { + PrivKey crypto.PrivKey + Config *PeerConfig + addr *NetAddress + quit chan struct{} + channels cmn.HexBytes +} + +func (rp *remotePeer) Addr() *NetAddress { + return rp.addr +} + +func (rp *remotePeer) ID() ID { + return PubKeyToID(rp.PrivKey.PubKey()) +} + +func (rp *remotePeer) Start() { + l, e := net.Listen("tcp", "127.0.0.1:0") // any available address + if e != nil { + golog.Fatalf("net.Listen tcp :0: %+v", e) + } + rp.addr = NewNetAddress(PubKeyToID(rp.PrivKey.PubKey()), l.Addr()) + rp.quit = make(chan struct{}) + if rp.channels == nil { + rp.channels = []byte{testCh} + } + go rp.accept(l) +} + +func (rp *remotePeer) Stop() { + close(rp.quit) +} + +func (rp *remotePeer) accept(l net.Listener) { + conns := []net.Conn{} + + for { + conn, err := l.Accept() + if err != nil { + golog.Fatalf("Failed to accept conn: %+v", err) + } + + pc, err := newInboundPeerConn(conn, rp.Config, rp.PrivKey) + if err != nil { + golog.Fatalf("Failed to create a peer: %+v", err) + } + + _, err = pc.HandshakeTimeout(NodeInfo{ + ID: rp.Addr().ID, + Moniker: "remote_peer", + Network: "localhost", + Version: "123.123.123", + ListenAddr: l.Addr().String(), + Channels: rp.channels, + }, 1*time.Second) + if err != nil { + golog.Fatalf("Failed to perform handshake: %+v", err) + } + + conns = append(conns, conn) + + select { + case <-rp.quit: + for _, conn := range conns { + if err := conn.Close(); err != nil { + golog.Fatal(err) + } + } + return + default: + } + } +} From d596ed1bc2ac9f0260b67b87c7d8a36216aba649 Mon Sep 17 00:00:00 2001 From: Alexander Simmerl Date: Fri, 18 May 2018 16:27:57 +0200 Subject: [PATCH 05/14] Let peerConn handle IPs in for tests --- p2p/peer.go | 12 +++++ p2p/peer_test.go | 79 +++++++++++++++++++++++++++++++++ p2p/test_util.go | 111 +---------------------------------------------- 3 files changed, 92 insertions(+), 110 deletions(-) diff --git a/p2p/peer.go b/p2p/peer.go index 2cb3d9f9f..93d05410e 100644 --- a/p2p/peer.go +++ b/p2p/peer.go @@ -12,6 +12,8 @@ import ( tmconn "github.com/tendermint/tendermint/p2p/conn" ) +var testIPSuffix = 0 + // Peer is an interface representing a peer connected on a reactor. type Peer interface { cmn.Service @@ -53,6 +55,16 @@ func (pc peerConn) RemoteIP() net.IP { return pc.ips[0] } + if pc.conn.RemoteAddr().String() == "pipe" { + pc.ips = []net.IP{ + net.IP{172, 16, 0, byte(testIPSuffix)}, + } + + testIPSuffix++ + + return pc.ips[0] + } + host, _, err := net.SplitHostPort(pc.conn.RemoteAddr().String()) if err != nil { panic(err) diff --git a/p2p/peer_test.go b/p2p/peer_test.go index a93b2ceea..db4f63ed7 100644 --- a/p2p/peer_test.go +++ b/p2p/peer_test.go @@ -1,6 +1,8 @@ package p2p import ( + golog "log" + "net" "testing" "time" @@ -9,9 +11,12 @@ import ( crypto "github.com/tendermint/go-crypto" tmconn "github.com/tendermint/tendermint/p2p/conn" + cmn "github.com/tendermint/tmlibs/common" "github.com/tendermint/tmlibs/log" ) +const testCh = 0x01 + func TestPeerBasic(t *testing.T) { assert, require := assert.New(t), require.New(t) @@ -105,3 +110,77 @@ func createOutboundPeerAndPerformHandshake(addr *NetAddress, config *PeerConfig) p.SetLogger(log.TestingLogger().With("peer", addr)) return p, nil } + +type remotePeer struct { + PrivKey crypto.PrivKey + Config *PeerConfig + addr *NetAddress + quit chan struct{} + channels cmn.HexBytes +} + +func (rp *remotePeer) Addr() *NetAddress { + return rp.addr +} + +func (rp *remotePeer) ID() ID { + return PubKeyToID(rp.PrivKey.PubKey()) +} + +func (rp *remotePeer) Start() { + l, e := net.Listen("tcp", "127.0.0.1:0") // any available address + if e != nil { + golog.Fatalf("net.Listen tcp :0: %+v", e) + } + rp.addr = NewNetAddress(PubKeyToID(rp.PrivKey.PubKey()), l.Addr()) + rp.quit = make(chan struct{}) + if rp.channels == nil { + rp.channels = []byte{testCh} + } + go rp.accept(l) +} + +func (rp *remotePeer) Stop() { + close(rp.quit) +} + +func (rp *remotePeer) accept(l net.Listener) { + conns := []net.Conn{} + + for { + conn, err := l.Accept() + if err != nil { + golog.Fatalf("Failed to accept conn: %+v", err) + } + + pc, err := newInboundPeerConn(conn, rp.Config, rp.PrivKey) + if err != nil { + golog.Fatalf("Failed to create a peer: %+v", err) + } + + _, err = pc.HandshakeTimeout(NodeInfo{ + ID: rp.Addr().ID, + Moniker: "remote_peer", + Network: "testing", + Version: "123.123.123", + ListenAddr: l.Addr().String(), + Channels: rp.channels, + }, 1*time.Second) + if err != nil { + golog.Fatalf("Failed to perform handshake: %+v", err) + } + + conns = append(conns, conn) + + select { + case <-rp.quit: + for _, conn := range conns { + if err := conn.Close(); err != nil { + golog.Fatal(err) + } + } + return + default: + } + } +} diff --git a/p2p/test_util.go b/p2p/test_util.go index de9243499..a0b3a5b88 100644 --- a/p2p/test_util.go +++ b/p2p/test_util.go @@ -1,10 +1,7 @@ package p2p import ( - "fmt" - golog "log" "net" - "time" crypto "github.com/tendermint/go-crypto" cmn "github.com/tendermint/tmlibs/common" @@ -14,8 +11,6 @@ import ( "github.com/tendermint/tendermint/p2p/conn" ) -const testCh = 0x01 - func AddPeerToSwitch(sw *Switch, peer Peer) { sw.peers.Add(peer) } @@ -86,37 +81,7 @@ func Connect2Switches(switches []*Switch, i, j int) { switchI := switches[i] switchJ := switches[j] - p1 := &remotePeer{ - Config: switchJ.peerConfig, - PrivKey: switchJ.nodeKey.PrivKey, - channels: switchJ.NodeInfo().Channels, - } - p1.Start() - - c1, err := net.DialTimeout( - "tcp", - fmt.Sprintf("%s:%d", p1.addr.IP.String(), p1.addr.Port), - 100*time.Millisecond, - ) - if err != nil { - panic(err) - } - - p2 := &remotePeer{ - Config: switchI.peerConfig, - PrivKey: switchI.nodeKey.PrivKey, - channels: switchI.NodeInfo().Channels, - } - p2.Start() - - c2, err := net.DialTimeout( - "tcp", - fmt.Sprintf("%s:%d", p2.addr.IP.String(), p2.addr.Port), - 100*time.Millisecond, - ) - if err != nil { - panic(err) - } + c1, c2 := conn.NetPipe() doneCh := make(chan struct{}) go func() { @@ -188,77 +153,3 @@ func MakeSwitch(cfg *cfg.P2PConfig, i int, network, version string, initSwitch f sw.SetNodeKey(nodeKey) return sw } - -type remotePeer struct { - PrivKey crypto.PrivKey - Config *PeerConfig - addr *NetAddress - quit chan struct{} - channels cmn.HexBytes -} - -func (rp *remotePeer) Addr() *NetAddress { - return rp.addr -} - -func (rp *remotePeer) ID() ID { - return PubKeyToID(rp.PrivKey.PubKey()) -} - -func (rp *remotePeer) Start() { - l, e := net.Listen("tcp", "127.0.0.1:0") // any available address - if e != nil { - golog.Fatalf("net.Listen tcp :0: %+v", e) - } - rp.addr = NewNetAddress(PubKeyToID(rp.PrivKey.PubKey()), l.Addr()) - rp.quit = make(chan struct{}) - if rp.channels == nil { - rp.channels = []byte{testCh} - } - go rp.accept(l) -} - -func (rp *remotePeer) Stop() { - close(rp.quit) -} - -func (rp *remotePeer) accept(l net.Listener) { - conns := []net.Conn{} - - for { - conn, err := l.Accept() - if err != nil { - golog.Fatalf("Failed to accept conn: %+v", err) - } - - pc, err := newInboundPeerConn(conn, rp.Config, rp.PrivKey) - if err != nil { - golog.Fatalf("Failed to create a peer: %+v", err) - } - - _, err = pc.HandshakeTimeout(NodeInfo{ - ID: rp.Addr().ID, - Moniker: "remote_peer", - Network: "localhost", - Version: "123.123.123", - ListenAddr: l.Addr().String(), - Channels: rp.channels, - }, 1*time.Second) - if err != nil { - golog.Fatalf("Failed to perform handshake: %+v", err) - } - - conns = append(conns, conn) - - select { - case <-rp.quit: - for _, conn := range conns { - if err := conn.Close(); err != nil { - golog.Fatal(err) - } - } - return - default: - } - } -} From 0cd92a494855e02cbd0f0a36c3c4e949a0b5d946 Mon Sep 17 00:00:00 2001 From: Alexander Simmerl Date: Mon, 21 May 2018 17:35:49 +0200 Subject: [PATCH 06/14] Fix race in test suffix --- p2p/peer.go | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/p2p/peer.go b/p2p/peer.go index 93d05410e..e80716268 100644 --- a/p2p/peer.go +++ b/p2p/peer.go @@ -3,6 +3,7 @@ package p2p import ( "fmt" "net" + "sync/atomic" "time" "github.com/tendermint/go-crypto" @@ -12,7 +13,7 @@ import ( tmconn "github.com/tendermint/tendermint/p2p/conn" ) -var testIPSuffix = 0 +var testIPSuffix uint32 = 0 // Peer is an interface representing a peer connected on a reactor. type Peer interface { @@ -57,11 +58,9 @@ func (pc peerConn) RemoteIP() net.IP { if pc.conn.RemoteAddr().String() == "pipe" { pc.ips = []net.IP{ - net.IP{172, 16, 0, byte(testIPSuffix)}, + net.IP{172, 16, 0, byte(atomic.AddUint32(&testIPSuffix, 1))}, } - testIPSuffix++ - return pc.ips[0] } From 7b02b5b66b436505ec680e882163804678ae0906 Mon Sep 17 00:00:00 2001 From: Alexander Simmerl Date: Mon, 21 May 2018 17:41:34 +0200 Subject: [PATCH 07/14] Add RemoteIP to test implementation --- blockchain/reactor_test.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/blockchain/reactor_test.go b/blockchain/reactor_test.go index 63e3c72bb..49913c10e 100644 --- a/blockchain/reactor_test.go +++ b/blockchain/reactor_test.go @@ -1,6 +1,7 @@ package blockchain import ( + "net" "testing" cmn "github.com/tendermint/tmlibs/common" @@ -204,3 +205,4 @@ func (tp *bcrTestPeer) IsOutbound() bool { return false } 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} } From 20e9dd07378be8996fb4385ab72741629af56071 Mon Sep 17 00:00:00 2001 From: Alexander Simmerl Date: Mon, 21 May 2018 17:55:40 +0200 Subject: [PATCH 08/14] Return fake IP even when there is no conn --- p2p/peer.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/p2p/peer.go b/p2p/peer.go index e80716268..39656efc5 100644 --- a/p2p/peer.go +++ b/p2p/peer.go @@ -56,7 +56,7 @@ func (pc peerConn) RemoteIP() net.IP { return pc.ips[0] } - if pc.conn.RemoteAddr().String() == "pipe" { + if pc.conn == nil || pc.conn.RemoteAddr().String() == "pipe" { pc.ips = []net.IP{ net.IP{172, 16, 0, byte(atomic.AddUint32(&testIPSuffix, 1))}, } From 91b6d3f18c8614365bedc2d28ad2047cf88e3218 Mon Sep 17 00:00:00 2001 From: Alexander Simmerl Date: Mon, 21 May 2018 18:47:14 +0200 Subject: [PATCH 09/14] Do not set address for self error --- p2p/switch.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/p2p/switch.go b/p2p/switch.go index 22307bd91..eba8679e7 100644 --- a/p2p/switch.go +++ b/p2p/switch.go @@ -570,7 +570,7 @@ func (sw *Switch) addPeer(pc peerConn) error { // and add to our addresses to avoid dialing again sw.addrBook.RemoveAddress(addr) sw.addrBook.AddOurAddress(addr) - return ErrSwitchConnectToSelf{addr} + return ErrSwitchConnectToSelf{} } // Avoid duplicate From 4848e88737eecdc31ec88a6c0b00832135b0042d Mon Sep 17 00:00:00 2001 From: Alexander Simmerl Date: Wed, 23 May 2018 00:24:40 +0200 Subject: [PATCH 10/14] Fix persistent peer switch test --- p2p/peer_test.go | 17 +++++++++++------ p2p/switch_test.go | 8 +++++++- 2 files changed, 18 insertions(+), 7 deletions(-) diff --git a/p2p/peer_test.go b/p2p/peer_test.go index db4f63ed7..22913f2de 100644 --- a/p2p/peer_test.go +++ b/p2p/peer_test.go @@ -112,11 +112,12 @@ func createOutboundPeerAndPerformHandshake(addr *NetAddress, config *PeerConfig) } type remotePeer struct { - PrivKey crypto.PrivKey - Config *PeerConfig - addr *NetAddress - quit chan struct{} - channels cmn.HexBytes + PrivKey crypto.PrivKey + Config *PeerConfig + addr *NetAddress + quit chan struct{} + channels cmn.HexBytes + listenAddr string } func (rp *remotePeer) Addr() *NetAddress { @@ -128,7 +129,11 @@ func (rp *remotePeer) ID() ID { } func (rp *remotePeer) Start() { - l, e := net.Listen("tcp", "127.0.0.1:0") // any available address + if rp.listenAddr == "" { + rp.listenAddr = "127.0.0.1:0" + } + + l, e := net.Listen("tcp", rp.listenAddr) // any available address if e != nil { golog.Fatalf("net.Listen tcp :0: %+v", e) } diff --git a/p2p/switch_test.go b/p2p/switch_test.go index 373378d73..14b018297 100644 --- a/p2p/switch_test.go +++ b/p2p/switch_test.go @@ -317,7 +317,13 @@ func TestSwitchReconnectsToPersistentPeer(t *testing.T) { assert.False(peer.IsRunning()) // simulate another remote peer - rp = &remotePeer{PrivKey: crypto.GenPrivKeyEd25519(), Config: DefaultPeerConfig()} + rp = &remotePeer{ + PrivKey: crypto.GenPrivKeyEd25519(), + Config: DefaultPeerConfig(), + // Use different interface to prevent duplicate IP filter, this will break + // beyond two peers. + listenAddr: "0.0.0.0:0", + } rp.Start() defer rp.Stop() From 7d98cfd3d6d3788a70df5db7167abc017d45ed6b Mon Sep 17 00:00:00 2001 From: Alexander Simmerl Date: Wed, 23 May 2018 01:24:27 +0200 Subject: [PATCH 11/14] Test duplicate IP guard in peer set --- p2p/peer_set_test.go | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/p2p/peer_set_test.go b/p2p/peer_set_test.go index ff2bdbad5..b043a36b2 100644 --- a/p2p/peer_set_test.go +++ b/p2p/peer_set_test.go @@ -146,6 +146,17 @@ func TestPeerSetAddDuplicate(t *testing.T) { } func TestPeerSetAddDuplicateIP(t *testing.T) { + t.Parallel() + + peerSet := NewPeerSet() + + if err := peerSet.Add(randPeer(net.IP{172, 0, 0, 1})); err != nil { + t.Fatal(err) + } + + // Add peer with same IP. + err := peerSet.Add(randPeer(net.IP{172, 0, 0, 1})) + assert.Equal(t, ErrSwitchDuplicatePeerIP{IP: net.IP{172, 0, 0, 1}}, err) } func TestPeerSetGet(t *testing.T) { From e11f3167ff78f84ae0d07bf4a3b666f59da36d16 Mon Sep 17 00:00:00 2001 From: Alexander Simmerl Date: Wed, 23 May 2018 01:35:03 +0200 Subject: [PATCH 12/14] Fix pex reactor test --- p2p/pex/pex_reactor_test.go | 2 +- p2p/switch_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/p2p/pex/pex_reactor_test.go b/p2p/pex/pex_reactor_test.go index 38aa84059..68443e74b 100644 --- a/p2p/pex/pex_reactor_test.go +++ b/p2p/pex/pex_reactor_test.go @@ -73,7 +73,7 @@ func TestPEXReactorRunning(t *testing.T) { // create switches for i := 0; i < N; i++ { - switches[i] = p2p.MakeSwitch(config, i, "127.0.0.1", "123.123.123", func(i int, sw *p2p.Switch) *p2p.Switch { + switches[i] = p2p.MakeSwitch(config, i, "127.0.0.2", "123.123.123", func(i int, sw *p2p.Switch) *p2p.Switch { books[i] = NewAddrBook(filepath.Join(dir, fmt.Sprintf("addrbook%d.json", i)), false) books[i].SetLogger(logger.With("pex", i)) sw.SetAddrBook(books[i]) diff --git a/p2p/switch_test.go b/p2p/switch_test.go index 14b018297..2c59d13e4 100644 --- a/p2p/switch_test.go +++ b/p2p/switch_test.go @@ -322,7 +322,7 @@ func TestSwitchReconnectsToPersistentPeer(t *testing.T) { Config: DefaultPeerConfig(), // Use different interface to prevent duplicate IP filter, this will break // beyond two peers. - listenAddr: "0.0.0.0:0", + listenAddr: "127.0.0.2:0", } rp.Start() defer rp.Stop() From 01fd102dba892973f786d589194ae96e01b3f4bb Mon Sep 17 00:00:00 2001 From: Alexander Simmerl Date: Wed, 23 May 2018 01:53:37 +0200 Subject: [PATCH 13/14] Incoporate review feedback --- p2p/peer.go | 18 +++++++++--------- p2p/peer_set_test.go | 4 +--- p2p/switch.go | 7 +++---- 3 files changed, 13 insertions(+), 16 deletions(-) diff --git a/p2p/peer.go b/p2p/peer.go index 39656efc5..447225bfb 100644 --- a/p2p/peer.go +++ b/p2p/peer.go @@ -41,7 +41,7 @@ type peerConn struct { persistent bool config *PeerConfig conn net.Conn // source connection - ips []net.IP + ip net.IP } // ID only exists for SecretConnection. @@ -52,16 +52,16 @@ func (pc peerConn) ID() ID { // Return the IP from the connection RemoteAddr func (pc peerConn) RemoteIP() net.IP { - if len(pc.ips) > 0 { - return pc.ips[0] + if pc.ip != nil { + return pc.ip } + // In test cases a conn could not be present at all or be an in-memory + // implementation where we want to return a fake ip. if pc.conn == nil || pc.conn.RemoteAddr().String() == "pipe" { - pc.ips = []net.IP{ - net.IP{172, 16, 0, byte(atomic.AddUint32(&testIPSuffix, 1))}, - } + pc.ip = net.IP{172, 16, 0, byte(atomic.AddUint32(&testIPSuffix, 1))} - return pc.ips[0] + return pc.ip } host, _, err := net.SplitHostPort(pc.conn.RemoteAddr().String()) @@ -74,9 +74,9 @@ func (pc peerConn) RemoteIP() net.IP { panic(err) } - pc.ips = ips + pc.ip = ips[0] - return ips[0] + return pc.ip } // peer implements Peer. diff --git a/p2p/peer_set_test.go b/p2p/peer_set_test.go index b043a36b2..fc3004684 100644 --- a/p2p/peer_set_test.go +++ b/p2p/peer_set_test.go @@ -26,9 +26,7 @@ func randPeer(ip net.IP) *peer { }, } - p.ips = []net.IP{ - ip, - } + p.ip = ip return p } diff --git a/p2p/switch.go b/p2p/switch.go index eba8679e7..6ea7e408f 100644 --- a/p2p/switch.go +++ b/p2p/switch.go @@ -534,8 +534,6 @@ func (sw *Switch) addPeer(pc peerConn) error { return err } - // dont connect to multiple peers on the same IP - // NOTE: if AuthEnc==false, we don't have a peerID until after the handshake. // If AuthEnc==true then we already know the ID and could do the checks first before the handshake, // but it's simple to just deal with both cases the same after the handshake. @@ -578,8 +576,9 @@ func (sw *Switch) addPeer(pc peerConn) error { return ErrSwitchDuplicatePeerID{peerID} } - // check ips for both the connection addr and the self reported addr - if sw.peers.HasIP(pc.RemoteIP()) { + // Check for duplicate connection or peer info IP. + if sw.peers.HasIP(pc.RemoteIP()) || + sw.peers.HasIP(peerNodeInfo.NetAddress().IP) { return ErrSwitchDuplicatePeerIP{pc.RemoteIP()} } From 186d38dd8a1e56afad56b74c4a489c234f646376 Mon Sep 17 00:00:00 2001 From: Alexander Simmerl Date: Wed, 23 May 2018 02:36:48 +0200 Subject: [PATCH 14/14] Use different loopback addresses for test switch --- p2p/pex/pex_reactor_test.go | 2 +- p2p/test_util.go | 6 +++++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/p2p/pex/pex_reactor_test.go b/p2p/pex/pex_reactor_test.go index 68443e74b..fc40f6fa0 100644 --- a/p2p/pex/pex_reactor_test.go +++ b/p2p/pex/pex_reactor_test.go @@ -73,7 +73,7 @@ func TestPEXReactorRunning(t *testing.T) { // create switches for i := 0; i < N; i++ { - switches[i] = p2p.MakeSwitch(config, i, "127.0.0.2", "123.123.123", func(i int, sw *p2p.Switch) *p2p.Switch { + switches[i] = p2p.MakeSwitch(config, i, "testing", "123.123.123", func(i int, sw *p2p.Switch) *p2p.Switch { books[i] = NewAddrBook(filepath.Join(dir, fmt.Sprintf("addrbook%d.json", i)), false) books[i].SetLogger(logger.With("pex", i)) sw.SetAddrBook(books[i]) diff --git a/p2p/test_util.go b/p2p/test_util.go index a0b3a5b88..86955f692 100644 --- a/p2p/test_util.go +++ b/p2p/test_util.go @@ -1,7 +1,9 @@ package p2p import ( + "fmt" "net" + "sync/atomic" crypto "github.com/tendermint/go-crypto" cmn "github.com/tendermint/tmlibs/common" @@ -130,6 +132,8 @@ func StartSwitches(switches []*Switch) error { return nil } +var listenAddrSuffix uint32 = 1 + func MakeSwitch(cfg *cfg.P2PConfig, i int, network, version string, initSwitch func(int, *Switch) *Switch) *Switch { // new switch, add reactors // TODO: let the config be passed in? @@ -144,7 +148,7 @@ func MakeSwitch(cfg *cfg.P2PConfig, i int, network, version string, initSwitch f Moniker: cmn.Fmt("switch%d", i), Network: network, Version: version, - ListenAddr: cmn.Fmt("%v:%v", network, cmn.RandIntn(64512)+1023), + ListenAddr: fmt.Sprintf("127.0.0.%d:%d", atomic.AddUint32(&listenAddrSuffix, 1), cmn.RandIntn(64512)+1023), } for ch := range sw.reactorsByCh { ni.Channels = append(ni.Channels, ch)