diff --git a/node/node.go b/node/node.go index 3a59d2f06..8b7ca1032 100644 --- a/node/node.go +++ b/node/node.go @@ -414,9 +414,14 @@ func (n *Node) OnStart() error { } n.Logger.Info("P2P Node ID", "ID", nodeKey.ID(), "file", n.config.NodeKeyFile()) - // Start the switch - n.sw.SetNodeInfo(n.makeNodeInfo(nodeKey.PubKey())) + nodeInfo := n.makeNodeInfo(nodeKey.PubKey()) + n.sw.SetNodeInfo(nodeInfo) n.sw.SetNodeKey(nodeKey) + + // Add ourselves to addrbook to prevent dialing ourselves + n.addrBook.AddOurAddress(nodeInfo.NetAddress()) + + // Start the switch err = n.sw.Start() if err != nil { return err diff --git a/p2p/pex/addrbook.go b/p2p/pex/addrbook.go index efb48f6da..adaba2fd6 100644 --- a/p2p/pex/addrbook.go +++ b/p2p/pex/addrbook.go @@ -34,6 +34,9 @@ type AddrBook interface { // Add our own addresses so we don't later add ourselves AddOurAddress(*p2p.NetAddress) + // Check if it is our address + OurAddress(*p2p.NetAddress) bool + // Add and remove an address AddAddress(addr *p2p.NetAddress, src *p2p.NetAddress) error RemoveAddress(addr *p2p.NetAddress) @@ -78,7 +81,7 @@ type addrBook struct { // accessed concurrently mtx sync.Mutex rand *rand.Rand - ourAddrs map[string]*p2p.NetAddress + ourAddrs map[string]struct{} addrLookup map[p2p.ID]*knownAddress // new & old bucketsOld []map[string]*knownAddress bucketsNew []map[string]*knownAddress @@ -93,7 +96,7 @@ type addrBook struct { func NewAddrBook(filePath string, routabilityStrict bool) *addrBook { am := &addrBook{ rand: rand.New(rand.NewSource(time.Now().UnixNano())), // TODO: seed from outside - ourAddrs: make(map[string]*p2p.NetAddress), + ourAddrs: make(map[string]struct{}), addrLookup: make(map[p2p.ID]*knownAddress), filePath: filePath, routabilityStrict: routabilityStrict, @@ -154,7 +157,15 @@ func (a *addrBook) AddOurAddress(addr *p2p.NetAddress) { a.mtx.Lock() defer a.mtx.Unlock() a.Logger.Info("Add our address to book", "addr", addr) - a.ourAddrs[addr.String()] = addr + a.ourAddrs[addr.String()] = struct{}{} +} + +// OurAddress returns true if it is our address. +func (a *addrBook) OurAddress(addr *p2p.NetAddress) bool { + a.mtx.Lock() + _, ok := a.ourAddrs[addr.String()] + a.mtx.Unlock() + return ok } // AddAddress implements AddrBook - adds the given address as received from the given source. diff --git a/p2p/pex/pex_reactor_test.go b/p2p/pex/pex_reactor_test.go index 20276ec86..329d17f37 100644 --- a/p2p/pex/pex_reactor_test.go +++ b/p2p/pex/pex_reactor_test.go @@ -63,35 +63,45 @@ func TestPEXReactorRunning(t *testing.T) { N := 3 switches := make([]*p2p.Switch, N) + // directory to store address books dir, err := ioutil.TempDir("", "pex_reactor") require.Nil(t, err) defer os.RemoveAll(dir) // nolint: errcheck - book := NewAddrBook(filepath.Join(dir, "addrbook.json"), false) - book.SetLogger(log.TestingLogger()) + + books := make([]*addrBook, N) + logger := log.TestingLogger() // 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 { - sw.SetLogger(log.TestingLogger().With("switch", i)) + books[i] = NewAddrBook(filepath.Join(dir, fmt.Sprintf("addrbook%d.json", i)), false) + books[i].SetLogger(logger.With("pex", i)) + sw.SetAddrBook(books[i]) - r := NewPEXReactor(book, &PEXReactorConfig{}) - r.SetLogger(log.TestingLogger()) + sw.SetLogger(logger.With("pex", i)) + + r := NewPEXReactor(books[i], &PEXReactorConfig{}) + r.SetLogger(logger.With("pex", i)) r.SetEnsurePeersPeriod(250 * time.Millisecond) sw.AddReactor("pex", r) + return sw }) } - // fill the address book and add listeners - for _, s := range switches { - addr := s.NodeInfo().NetAddress() - book.AddAddress(addr, addr) - s.AddListener(p2p.NewDefaultListener("tcp", s.NodeInfo().ListenAddr, true, log.TestingLogger())) + addOtherNodeAddrToAddrBook := func(switchIndex, otherSwitchIndex int) { + addr := switches[otherSwitchIndex].NodeInfo().NetAddress() + books[switchIndex].AddAddress(addr, addr) } - // start switches - for _, s := range switches { - err := s.Start() // start switch and reactors + addOtherNodeAddrToAddrBook(0, 1) + addOtherNodeAddrToAddrBook(1, 0) + addOtherNodeAddrToAddrBook(2, 1) + + for i, sw := range switches { + sw.AddListener(p2p.NewDefaultListener("tcp", sw.NodeInfo().ListenAddr, true, logger.With("pex", i))) + + err := sw.Start() // start switch and reactors require.Nil(t, err) } @@ -127,6 +137,7 @@ func TestPEXReactorRequestMessageAbuse(t *testing.T) { defer teardownReactor(book) sw := createSwitchAndAddReactors(r) + sw.SetAddrBook(book) peer := newMockPeer() p2p.AddPeerToSwitch(sw, peer) @@ -156,6 +167,7 @@ func TestPEXReactorAddrsMessageAbuse(t *testing.T) { defer teardownReactor(book) sw := createSwitchAndAddReactors(r) + sw.SetAddrBook(book) peer := newMockPeer() p2p.AddPeerToSwitch(sw, peer) @@ -182,13 +194,11 @@ func TestPEXReactorAddrsMessageAbuse(t *testing.T) { } func TestPEXReactorUsesSeedsIfNeeded(t *testing.T) { + // directory to store address books dir, err := ioutil.TempDir("", "pex_reactor") require.Nil(t, err) defer os.RemoveAll(dir) // nolint: errcheck - book := NewAddrBook(filepath.Join(dir, "addrbook.json"), false) - book.SetLogger(log.TestingLogger()) - // 1. create seed seed := p2p.MakeSwitch( config, @@ -196,6 +206,10 @@ func TestPEXReactorUsesSeedsIfNeeded(t *testing.T) { "127.0.0.1", "123.123.123", func(i int, sw *p2p.Switch) *p2p.Switch { + book := NewAddrBook(filepath.Join(dir, "addrbook0.json"), false) + book.SetLogger(log.TestingLogger()) + sw.SetAddrBook(book) + sw.SetLogger(log.TestingLogger()) r := NewPEXReactor(book, &PEXReactorConfig{}) @@ -222,6 +236,10 @@ func TestPEXReactorUsesSeedsIfNeeded(t *testing.T) { "127.0.0.1", "123.123.123", func(i int, sw *p2p.Switch) *p2p.Switch { + book := NewAddrBook(filepath.Join(dir, "addrbook1.json"), false) + book.SetLogger(log.TestingLogger()) + sw.SetAddrBook(book) + sw.SetLogger(log.TestingLogger()) r := NewPEXReactor( @@ -247,7 +265,8 @@ func TestPEXReactorCrawlStatus(t *testing.T) { defer teardownReactor(book) // Seed/Crawler mode uses data from the Switch - _ = createSwitchAndAddReactors(pexR) + sw := createSwitchAndAddReactors(pexR) + sw.SetAddrBook(book) // Create a peer, add it to the peer set and the addrbook. peer := p2p.CreateRandomPeer(false) @@ -291,7 +310,8 @@ func TestPEXReactorDialPeer(t *testing.T) { pexR, book := createReactor(&PEXReactorConfig{}) defer teardownReactor(book) - _ = createSwitchAndAddReactors(pexR) + sw := createSwitchAndAddReactors(pexR) + sw.SetAddrBook(book) peer := newMockPeer() addr := peer.NodeInfo().NetAddress() @@ -397,6 +417,7 @@ func assertPeersWithTimeout( } func createReactor(config *PEXReactorConfig) (r *PEXReactor, book *addrBook) { + // directory to store address book dir, err := ioutil.TempDir("", "pex_reactor") if err != nil { panic(err) diff --git a/p2p/switch.go b/p2p/switch.go index fec448265..a12052953 100644 --- a/p2p/switch.go +++ b/p2p/switch.go @@ -35,6 +35,8 @@ const ( type AddrBook interface { AddAddress(addr *NetAddress, src *NetAddress) error + AddOurAddress(*NetAddress) + OurAddress(*NetAddress) bool MarkGood(*NetAddress) Save() } @@ -48,18 +50,17 @@ type AddrBook interface { type Switch struct { cmn.BaseService - config *cfg.P2PConfig - peerConfig *PeerConfig - listeners []Listener - reactors map[string]Reactor - chDescs []*conn.ChannelDescriptor - reactorsByCh map[byte]Reactor - peers *PeerSet - dialing *cmn.CMap - nodeInfo NodeInfo // our node info - nodeKey *NodeKey // our node privkey - nodePublicAddr string - addrBook AddrBook + config *cfg.P2PConfig + peerConfig *PeerConfig + listeners []Listener + reactors map[string]Reactor + chDescs []*conn.ChannelDescriptor + reactorsByCh map[byte]Reactor + peers *PeerSet + dialing *cmn.CMap + nodeInfo NodeInfo // our node info + nodeKey *NodeKey // our node privkey + addrBook AddrBook filterConnByAddr func(net.Addr) error filterConnByID func(ID) error @@ -149,7 +150,6 @@ func (sw *Switch) IsListening() bool { // NOTE: Not goroutine safe. func (sw *Switch) SetNodeInfo(nodeInfo NodeInfo) { sw.nodeInfo = nodeInfo - sw.nodePublicAddr = nodeInfo.ListenAddr } // NodeInfo returns the switch's NodeInfo. @@ -384,7 +384,7 @@ func (sw *Switch) DialPeersAsync(addrBook AddrBook, peers []string, persistent b // If `persistent == true`, the switch will always try to reconnect to this peer if the connection ever fails. func (sw *Switch) DialPeerWithAddress(addr *NetAddress, persistent bool) error { // do not dial ourselves - if addr.DialString() == sw.nodePublicAddr { + if sw.addrBook.OurAddress(addr) { return ErrSwitchConnectToSelf } @@ -529,9 +529,9 @@ func (sw *Switch) addPeer(pc peerConn) error { // Avoid self if sw.nodeKey.ID() == peerID { - // overwrite current addr to avoid dialing ourselves again - // it means original nodePublicAddr was different from public one - sw.nodePublicAddr = peerNodeInfo.ListenAddr + // add given address to the address book to avoid dialing ourselves again + // this is our public address + sw.addrBook.AddOurAddress(peerNodeInfo.NetAddress()) return ErrSwitchConnectToSelf } diff --git a/p2p/switch_test.go b/p2p/switch_test.go index 7ac72395c..4995ad3f2 100644 --- a/p2p/switch_test.go +++ b/p2p/switch_test.go @@ -90,6 +90,8 @@ func MakeSwitchPair(t testing.TB, initSwitch func(int, *Switch) *Switch) (*Switc } func initSwitchFunc(i int, sw *Switch) *Switch { + sw.SetAddrBook(&addrBookMock{ourAddrs: make(map[string]struct{})}) + // Make two reactors of two channels each sw.AddReactor("foo", NewTestReactor([]*conn.ChannelDescriptor{ {ID: byte(0x00), Priority: 10}, @@ -99,6 +101,7 @@ func initSwitchFunc(i int, sw *Switch) *Switch { {ID: byte(0x02), Priority: 10}, {ID: byte(0x03), Priority: 10}, }, true)) + return sw } @@ -177,9 +180,12 @@ func TestConnAddrFilter(t *testing.T) { func TestSwitchFiltersOutItself(t *testing.T) { s1 := MakeSwitch(config, 1, "127.0.0.2", "123.123.123", initSwitchFunc) + addr := s1.NodeInfo().NetAddress() + + // add ourselves like we do in node.go#427 + s1.addrBook.AddOurAddress(addr) // addr should be rejected immediately because of the same IP & port - addr := s1.NodeInfo().NetAddress() err := s1.DialPeerWithAddress(addr, false) if assert.Error(t, err) { assert.Equal(t, ErrSwitchConnectToSelf, err) @@ -371,3 +377,18 @@ func BenchmarkSwitchBroadcast(b *testing.B) { b.Logf("success: %v, failure: %v", numSuccess, numFailure) } + +type addrBookMock struct { + ourAddrs map[string]struct{} +} + +var _ AddrBook = (*addrBookMock)(nil) + +func (book *addrBookMock) AddAddress(addr *NetAddress, src *NetAddress) error { return nil } +func (book *addrBookMock) AddOurAddress(addr *NetAddress) { book.ourAddrs[addr.String()] = struct{}{} } +func (book *addrBookMock) OurAddress(addr *NetAddress) bool { + _, ok := book.ourAddrs[addr.String()] + return ok +} +func (book *addrBookMock) MarkGood(*NetAddress) {} +func (book *addrBookMock) Save() {}