From 332f7056f721fb8bfd9359f2586e6b906a752613 Mon Sep 17 00:00:00 2001 From: Anton Kalyaev Date: Fri, 13 Jan 2017 19:33:31 +0400 Subject: [PATCH 1/6] start/stop the book with reactor Refs https://github.com/tendermint/tendermint/issues/335 --- pex_reactor.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/pex_reactor.go b/pex_reactor.go index c02d9a679..ed0ab76e8 100644 --- a/pex_reactor.go +++ b/pex_reactor.go @@ -42,12 +42,14 @@ func NewPEXReactor(book *AddrBook) *PEXReactor { func (pexR *PEXReactor) OnStart() error { pexR.BaseReactor.OnStart() + pexR.book.OnStart() go pexR.ensurePeersRoutine() return nil } func (pexR *PEXReactor) OnStop() { pexR.BaseReactor.OnStop() + pexR.book.OnStop() } // Implements Reactor From e7656873c1f290883d3272cc5182cbf2f8890abe Mon Sep 17 00:00:00 2001 From: Anton Kalyaev Date: Fri, 13 Jan 2017 20:48:42 +0400 Subject: [PATCH 2/6] public save API --- addrbook.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/addrbook.go b/addrbook.go index 2bbae64f5..c5415f022 100644 --- a/addrbook.go +++ b/addrbook.go @@ -368,6 +368,12 @@ func (a *AddrBook) loadFromFile(filePath string) bool { return true } +// Save saves the book. +func (a *AddrBook) Save() { + log.Info("Saving AddrBook to file", "size", a.Size()) + a.saveToFile(a.filePath) +} + /* Private methods */ func (a *AddrBook) saveRoutine() { From 2773410de43d48c7c1a49591ccdb194db355bd7d Mon Sep 17 00:00:00 2001 From: Anton Kaliaev Date: Mon, 16 Jan 2017 12:39:57 +0400 Subject: [PATCH 3/6] prevent nil addr Error: ``` Error: runtime error: invalid memoryaddress or nil pointer dereference\nStack: goroutine 549 [running]:\nruntime/debug.Stack(0x0, 0x0, 0x0)\n\t/usr/local/go/src/runtime/debug/stack.go:24 +0x80\ngithub.com/tendermint/tendermint/vendor/github.com/tendermint/go-p2p.(*MConnection)._recover(0xc821723b00)\n\t/go/src/github.com/tendermint/tendermint/vendor/github.com/tendermint/go-p2p/connection.go:173 +0x53\npanic(0xbe1500, 0xc820012080)\n\t/usr/local/go/src/runtime/panic.go:443 +0x4e9\ngithub.com/tendermint/tendermint/vendor/github.com/tendermint/go-p2p.(*NetAddress).Valid(0x0, 0x0)\n\t/go/src/github.com/tendermint/tendermint/vendor/github.com/tendermint/go-p2p/netaddress.go:125 +0x1c\ngithub.com/tendermint/tendermint/vendor/github.com/tendermint/go-p2p.(*NetAddress).Routable(0x0, 0xc8217bb740)\n\t/go/src/github.com/tendermint/tendermint/vendor/github.com/tendermint/go-p2p/netaddress.go:117 +0x25\ngithub.com/tendermint/tendermint/vendor/github.com/tendermint/go-p2p.(*AddrBook).addAddress(0xc820108380, 0x0, 0xc821739590)\n\t/go/src/github.com/tendermint/tendermint/vendor/github.com/tendermint/go-p2p/addrbook.go:524 +0x45\ngithub.com/tendermint/tendermint/vendor/github.com/tendermint/go-p2p.(*AddrBook).AddAddress(0xc820108380, 0x0, 0xc821739590)\n\t/go/src/github.com/tendermint/tendermint/vendor/github.com/tendermint/go-p2p/addrbook.go:160 +0x286\ngithub.com/tendermint/tendermint/vendor/github.com/tendermint/go-p2p.(*PEXReactor).Receive(0xc82000be60, 0xc820149f00, 0xc8218163f0, 0xc82184e000, 0x5b, 0x1000)\n\t/go/src/github.com/tendermint/tendermint/vendor/github.com/tendermint/go-p2p/pex_reactor.go:109 +0x457\ngithub.com/tendermint/tendermint/vendor/github.com/tendermint/go-p2p.newPeer.func1(0xc82011d500, 0xc82184e000, 0x5b, 0x1000)\n\t/go/src/github.com/tendermint/tendermint/vendor/github.com/tendermint/go-p2p/peer.go:58 +0x202\ngithub.com/tendermint/tendermint/vendor/github.com/tendermint/go-p2p.(*MConnection).recvRoutine(0xc821723b00)\n\t/go/src/github.com/tendermint/tendermint/vendor/github.com/tendermint/go-p2p/connection.go:439 +0x1177\ncreated by github.com/tendermint/tendermint/vendor/github.com/tendermint/go-p2p.(*MConnection).OnStart\n\t/go/src/github.com/tendermint/tendermint/vendor/github.com/tendermint/go-p2p/connection.go:138 +0x1a1\n ``` --- pex_reactor.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/pex_reactor.go b/pex_reactor.go index ed0ab76e8..b8cb43f80 100644 --- a/pex_reactor.go +++ b/pex_reactor.go @@ -112,7 +112,9 @@ func (pexR *PEXReactor) Receive(chID byte, src *Peer, msgBytes []byte) { // (We don't want to get spammed with bad peers) srcAddr := src.Connection().RemoteAddress for _, addr := range msg.Addrs { - pexR.book.AddAddress(addr, srcAddr) + if addr != nil { + pexR.book.AddAddress(addr, srcAddr) + } } default: log.Warn(Fmt("Unknown message type %v", reflect.TypeOf(msg))) From 108beae7a83092692ca079f1fff17b879e71fdc3 Mon Sep 17 00:00:00 2001 From: Anton Kaliaev Date: Mon, 16 Jan 2017 15:57:08 +0400 Subject: [PATCH 4/6] more tests for AddrBook --- addrbook_test.go | 170 +++++++++++++++++++++-------------------------- 1 file changed, 77 insertions(+), 93 deletions(-) diff --git a/addrbook_test.go b/addrbook_test.go index 5eb7c0b6a..7e8cb8d76 100644 --- a/addrbook_test.go +++ b/addrbook_test.go @@ -9,8 +9,6 @@ import ( "github.com/stretchr/testify/assert" ) -const addrBookStrict = true - func createTempFileName(prefix string) string { f, err := ioutil.TempFile("", prefix) if err != nil { @@ -24,143 +22,129 @@ func createTempFileName(prefix string) string { return fname } -func TestEmpty(t *testing.T) { +func TestAddrBookSaveLoad(t *testing.T) { fname := createTempFileName("addrbook_test") - // t.Logf("New tempfile name: %v", fname) - // Save an empty book & load it - book := NewAddrBook(fname, addrBookStrict) + // 0 addresses + book := NewAddrBook(fname, true) book.saveToFile(fname) - book = NewAddrBook(fname, addrBookStrict) + book = NewAddrBook(fname, true) book.loadFromFile(fname) - if book.Size() != 0 { - t.Errorf("Expected 0 addresses, found %v", book.Size()) - } -} + assert.Zero(t, book.Size()) -func randIPv4Address(t *testing.T) *NetAddress { - for { - ip := fmt.Sprintf("%v.%v.%v.%v", - rand.Intn(254)+1, - rand.Intn(255), - rand.Intn(255), - rand.Intn(255), - ) - port := rand.Intn(65535-1) + 1 - addr, err := NewNetAddressString(fmt.Sprintf("%v:%v", ip, port)) - assert.Nil(t, err, "error generating rand network address") - if addr.Routable() { - return addr - } - } -} + // 100 addresses + randAddrs := randNetAddressPairs(t, 100) -func TestSaveAddresses(t *testing.T) { - fname := createTempFileName("addrbook_test") - //t.Logf("New tempfile name: %v", fname) - - // Create some random addresses - randAddrs := []struct { - addr *NetAddress - src *NetAddress - }{} - for i := 0; i < 100; i++ { - addr := randIPv4Address(t) - src := randIPv4Address(t) - randAddrs = append(randAddrs, struct { - addr *NetAddress - src *NetAddress - }{ - addr: addr, - src: src, - }) - } - - // Create the book & populate & save - book := NewAddrBook(fname, addrBookStrict) for _, addrSrc := range randAddrs { book.AddAddress(addrSrc.addr, addrSrc.src) } - if book.Size() != 100 { - t.Errorf("Expected 100 addresses, found %v", book.Size()) - } + + assert.Equal(t, 100, book.Size()) book.saveToFile(fname) - // Reload the book - book = NewAddrBook(fname, addrBookStrict) + book = NewAddrBook(fname, true) book.loadFromFile(fname) - // Test ... + assert.Equal(t, 100, book.Size()) +} - if book.Size() != 100 { - t.Errorf("Expected 100 addresses, found %v", book.Size()) - } +func TestAddrBookLookup(t *testing.T) { + fname := createTempFileName("addrbook_test") + + randAddrs := randNetAddressPairs(t, 100) + book := NewAddrBook(fname, true) for _, addrSrc := range randAddrs { addr := addrSrc.addr src := addrSrc.src + book.AddAddress(addr, src) + ka := book.addrLookup[addr.String()] - if ka == nil { - t.Fatalf("Expected to find KnownAddress %v but wasn't there.", addr) - } + assert.NotNil(t, ka, "Expected to find KnownAddress %v but wasn't there.", addr) + if !(ka.Addr.Equals(addr) && ka.Src.Equals(src)) { t.Fatalf("KnownAddress doesn't match addr & src") } } } -func TestPromoteToOld(t *testing.T) { +func TestAddrBookPromoteToOld(t *testing.T) { fname := createTempFileName("addrbook_test") - t.Logf("New tempfile name: %v", fname) - - // Create some random addresses - randAddrs := []struct { - addr *NetAddress - src *NetAddress - }{} - for i := 0; i < 100; i++ { - addr := randIPv4Address(t) - src := randIPv4Address(t) - randAddrs = append(randAddrs, struct { - addr *NetAddress - src *NetAddress - }{ - addr: addr, - src: src, - }) - } - // Create the book & populate & save - book := NewAddrBook(fname, addrBookStrict) + randAddrs := randNetAddressPairs(t, 100) + + book := NewAddrBook(fname, true) for _, addrSrc := range randAddrs { book.AddAddress(addrSrc.addr, addrSrc.src) } + // Attempt all addresses. for _, addrSrc := range randAddrs { book.MarkAttempt(addrSrc.addr) } + // Promote half of them for i, addrSrc := range randAddrs { if i%2 == 0 { book.MarkGood(addrSrc.addr) } } - book.saveToFile(fname) - // Reload the book - book = NewAddrBook(fname, addrBookStrict) - book.loadFromFile(fname) + // TODO: do more testing :) - // Test ... + selection := book.GetSelection() + t.Logf("selection: %v", selection) - if book.Size() != 100 { - t.Errorf("Expected 100 addresses, found %v", book.Size()) + if len(selection) > book.Size() { + t.Errorf("selection could not be bigger than the book") } +} - // TODO: do more testing :) +func TestAddrBookHandlesDuplicates(t *testing.T) { + fname := createTempFileName("addrbook_test") - selection := book.GetSelection() - t.Logf("selection: %v", selection) + book := NewAddrBook(fname, true) + + randAddrs := randNetAddressPairs(t, 100) + + differentSrc := randIPv4Address(t) + for _, addrSrc := range randAddrs { + book.AddAddress(addrSrc.addr, addrSrc.src) + book.AddAddress(addrSrc.addr, addrSrc.src) // duplicate + book.AddAddress(addrSrc.addr, differentSrc) // different src + } + + assert.Equal(t, 100, book.Size()) +} + +type netAddressPair struct { + addr *NetAddress + src *NetAddress +} + +func randNetAddressPairs(t *testing.T, n int) []netAddressPair { + randAddrs := make([]netAddressPair, n) + for i := 0; i < n; i++ { + randAddrs[i] = netAddressPair{addr: randIPv4Address(t), src: randIPv4Address(t)} + } + return randAddrs +} + +func randIPv4Address(t *testing.T) *NetAddress { + for { + ip := fmt.Sprintf("%v.%v.%v.%v", + rand.Intn(254)+1, + rand.Intn(255), + rand.Intn(255), + rand.Intn(255), + ) + port := rand.Intn(65535-1) + 1 + addr, err := NewNetAddressString(fmt.Sprintf("%v:%v", ip, port)) + assert.Nil(t, err, "error generating rand network address") + if addr.Routable() { + return addr + } + } } From 65b1756978fb6fe1c247dd361e3ef56dbd4a5165 Mon Sep 17 00:00:00 2001 From: Anton Kaliaev Date: Mon, 16 Jan 2017 18:00:39 +0400 Subject: [PATCH 5/6] expose 2 API functions for tendermint#node/node.go --- pex_reactor.go | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/pex_reactor.go b/pex_reactor.go index b8cb43f80..2948c58f9 100644 --- a/pex_reactor.go +++ b/pex_reactor.go @@ -130,6 +130,16 @@ func (pexR *PEXReactor) SendAddrs(peer *Peer, addrs []*NetAddress) { peer.Send(PexChannel, struct{ PexMessage }{&pexAddrsMessage{Addrs: addrs}}) } +// SaveAddrBook saves underlying address book +func (r *PEXReactor) SaveAddrBook() { + r.book.Save() +} + +// AddPeerAddress adds raw NetAddress to the address book +func (r *PEXReactor) AddPeerAddress(peerAddr, srcAddr *NetAddress) { + r.book.AddAddress(peerAddr, srcAddr) +} + // Ensures that sufficient peers are connected. (continuous) func (pexR *PEXReactor) ensurePeersRoutine() { // Randomize when routine starts From 88b5c724f26f99d4094e2b8dcef72fb9bf89c5c4 Mon Sep 17 00:00:00 2001 From: Ethan Buchman Date: Sat, 4 Mar 2017 22:55:42 -0500 Subject: [PATCH 6/6] remove public addr book funcs from pex --- addrbook.go | 1 + pex_reactor.go | 10 ---------- 2 files changed, 1 insertion(+), 10 deletions(-) diff --git a/addrbook.go b/addrbook.go index c5415f022..ea438120b 100644 --- a/addrbook.go +++ b/addrbook.go @@ -153,6 +153,7 @@ func (a *AddrBook) OurAddresses() []*NetAddress { return addrs } +// NOTE: addr must not be nil func (a *AddrBook) AddAddress(addr *NetAddress, src *NetAddress) { a.mtx.Lock() defer a.mtx.Unlock() diff --git a/pex_reactor.go b/pex_reactor.go index 2948c58f9..b8cb43f80 100644 --- a/pex_reactor.go +++ b/pex_reactor.go @@ -130,16 +130,6 @@ func (pexR *PEXReactor) SendAddrs(peer *Peer, addrs []*NetAddress) { peer.Send(PexChannel, struct{ PexMessage }{&pexAddrsMessage{Addrs: addrs}}) } -// SaveAddrBook saves underlying address book -func (r *PEXReactor) SaveAddrBook() { - r.book.Save() -} - -// AddPeerAddress adds raw NetAddress to the address book -func (r *PEXReactor) AddPeerAddress(peerAddr, srcAddr *NetAddress) { - r.book.AddAddress(peerAddr, srcAddr) -} - // Ensures that sufficient peers are connected. (continuous) func (pexR *PEXReactor) ensurePeersRoutine() { // Randomize when routine starts