From 3863885c719f448694e26112ab1d309c1144f990 Mon Sep 17 00:00:00 2001 From: Petabyte Storage Date: Sun, 12 Nov 2017 22:11:15 -0800 Subject: [PATCH 01/16] WIP: begin parallel refactoring with go-wire Write methods and MConnection --- p2p/connection.go | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/p2p/connection.go b/p2p/connection.go index 30935c71e..11578eb8e 100644 --- a/p2p/connection.go +++ b/p2p/connection.go @@ -11,10 +11,13 @@ import ( "time" wire "github.com/tendermint/go-wire" + tmencoding "github.com/tendermint/go-wire/nowriter/tmencoding" cmn "github.com/tendermint/tmlibs/common" flow "github.com/tendermint/tmlibs/flowrate" ) +var legacy = tmencoding.Legacy + const ( numBatchMsgPackets = 10 minReadBufferSize = 1024 @@ -308,12 +311,12 @@ FOR_LOOP: } case <-c.pingTimer.Ch: c.Logger.Debug("Send Ping") - wire.WriteByte(packetTypePing, c.bufWriter, &n, &err) + legacy.WriteOctet(packetTypePing, c.bufWriter, &n, &err) c.sendMonitor.Update(int(n)) c.flush() case <-c.pong: c.Logger.Debug("Send Pong") - wire.WriteByte(packetTypePong, c.bufWriter, &n, &err) + legacy.WriteOctet(packetTypePong, c.bufWriter, &n, &err) c.sendMonitor.Update(int(n)) c.flush() case <-c.quit: @@ -661,7 +664,7 @@ func (ch *Channel) writeMsgPacketTo(w io.Writer) (n int, err error) { } func writeMsgPacketTo(packet msgPacket, w io.Writer, n *int, err *error) { - wire.WriteByte(packetTypeMsg, w, n, err) + legacy.WriteOctet(packetTypeMsg, w, n, err) wire.WriteBinary(packet, w, n, err) } From 498a82784d46284c393fe4ce612c8dbe74094e03 Mon Sep 17 00:00:00 2001 From: Ethan Buchman Date: Thu, 16 Nov 2017 02:25:00 +0000 Subject: [PATCH 02/16] p2p/addrbook: comments --- p2p/addrbook.go | 43 +++++++++++++++++++++++++++---------------- 1 file changed, 27 insertions(+), 16 deletions(-) diff --git a/p2p/addrbook.go b/p2p/addrbook.go index 06162e2e6..92b3e0d7e 100644 --- a/p2p/addrbook.go +++ b/p2p/addrbook.go @@ -40,7 +40,7 @@ const ( // old buckets over which an address group will be spread. oldBucketsPerGroup = 4 - // new buckets over which an source address group will be spread. + // new buckets over which a source address group will be spread. newBucketsPerGroup = 32 // buckets a frequently seen new address may end up in. @@ -79,18 +79,22 @@ const ( type AddrBook struct { cmn.BaseService - mtx sync.Mutex + // immutable after creation filePath string routabilityStrict bool - rand *rand.Rand key string - ourAddrs map[string]*NetAddress - addrLookup map[string]*knownAddress // new & old - addrNew []map[string]*knownAddress - addrOld []map[string]*knownAddress - wg sync.WaitGroup - nOld int - nNew int + + // accessed concurrently + mtx sync.Mutex + rand *rand.Rand + ourAddrs map[string]*NetAddress + addrLookup map[string]*knownAddress // new & old + addrNew []map[string]*knownAddress + addrOld []map[string]*knownAddress + nOld int + nNew int + + wg sync.WaitGroup } // NewAddrBook creates a new address book. @@ -145,6 +149,7 @@ func (a *AddrBook) Wait() { a.wg.Wait() } +// AddOurAddress adds another one of our addresses. func (a *AddrBook) AddOurAddress(addr *NetAddress) { a.mtx.Lock() defer a.mtx.Unlock() @@ -152,6 +157,7 @@ func (a *AddrBook) AddOurAddress(addr *NetAddress) { a.ourAddrs[addr.String()] = addr } +// OurAddresses returns a list of our addresses. func (a *AddrBook) OurAddresses() []*NetAddress { addrs := []*NetAddress{} for _, addr := range a.ourAddrs { @@ -160,6 +166,7 @@ func (a *AddrBook) OurAddresses() []*NetAddress { return addrs } +// AddAddress adds the given address as received from the given source. // NOTE: addr must not be nil func (a *AddrBook) AddAddress(addr *NetAddress, src *NetAddress) { a.mtx.Lock() @@ -168,10 +175,12 @@ func (a *AddrBook) AddAddress(addr *NetAddress, src *NetAddress) { a.addAddress(addr, src) } +// NeedMoreAddrs returns true if there are not have enough addresses in the book. func (a *AddrBook) NeedMoreAddrs() bool { return a.Size() < needAddressThreshold } +// Size returns the number of addresses in the book. func (a *AddrBook) Size() int { a.mtx.Lock() defer a.mtx.Unlock() @@ -182,7 +191,7 @@ func (a *AddrBook) size() int { return a.nNew + a.nOld } -// Pick an address to connect to with new/old bias. +// PickAddress picks an address to connect to with new/old bias. func (a *AddrBook) PickAddress(newBias int) *NetAddress { a.mtx.Lock() defer a.mtx.Unlock() @@ -201,9 +210,9 @@ func (a *AddrBook) PickAddress(newBias int) *NetAddress { oldCorrelation := math.Sqrt(float64(a.nOld)) * (100.0 - float64(newBias)) newCorrelation := math.Sqrt(float64(a.nNew)) * float64(newBias) - if (newCorrelation+oldCorrelation)*a.rand.Float64() < oldCorrelation { - // pick random Old bucket. - var bucket map[string]*knownAddress = nil + pickFromOldBucket := (newCorrelation+oldCorrelation)*a.rand.Float64() < oldCorrelation + if pickFromOldBucket { + var bucket map[string]*knownAddress for len(bucket) == 0 { bucket = a.addrOld[a.rand.Intn(len(a.addrOld))] } @@ -217,7 +226,6 @@ func (a *AddrBook) PickAddress(newBias int) *NetAddress { } cmn.PanicSanity("Should not happen") } else { - // pick random New bucket. var bucket map[string]*knownAddress = nil for len(bucket) == 0 { bucket = a.addrNew[a.rand.Intn(len(a.addrNew))] @@ -235,6 +243,7 @@ func (a *AddrBook) PickAddress(newBias int) *NetAddress { return nil } +// MarkGood marks the peer as good and moves it into an "old" bucket. func (a *AddrBook) MarkGood(addr *NetAddress) { a.mtx.Lock() defer a.mtx.Unlock() @@ -248,6 +257,7 @@ func (a *AddrBook) MarkGood(addr *NetAddress) { } } +// MarkAttempt marks that an attempt was made to connect to the address. func (a *AddrBook) MarkAttempt(addr *NetAddress) { a.mtx.Lock() defer a.mtx.Unlock() @@ -823,7 +833,8 @@ func (ka *knownAddress) isBad() bool { return false } - // Over a month old? + // Too old? + // XXX: does this mean if we've kept a connection up for this long we'll disconnect?! if ka.LastAttempt.After(time.Now().Add(-1 * numMissingDays * time.Hour * 24)) { return true } From 2f067a3f656c41846874f58ee890c49e02a73a96 Mon Sep 17 00:00:00 2001 From: Ethan Buchman Date: Thu, 16 Nov 2017 02:28:11 +0000 Subject: [PATCH 03/16] p2p/addrbook: addrNew/Old -> bucketsNew/Old --- p2p/addrbook.go | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/p2p/addrbook.go b/p2p/addrbook.go index 92b3e0d7e..eeeeaf5ee 100644 --- a/p2p/addrbook.go +++ b/p2p/addrbook.go @@ -89,8 +89,8 @@ type AddrBook struct { rand *rand.Rand ourAddrs map[string]*NetAddress addrLookup map[string]*knownAddress // new & old - addrNew []map[string]*knownAddress - addrOld []map[string]*knownAddress + bucketsOld []map[string]*knownAddress + bucketsNew []map[string]*knownAddress nOld int nNew int @@ -116,14 +116,14 @@ func NewAddrBook(filePath string, routabilityStrict bool) *AddrBook { func (a *AddrBook) init() { a.key = crypto.CRandHex(24) // 24/2 * 8 = 96 bits // New addr buckets - a.addrNew = make([]map[string]*knownAddress, newBucketCount) - for i := range a.addrNew { - a.addrNew[i] = make(map[string]*knownAddress) + a.bucketsNew = make([]map[string]*knownAddress, newBucketCount) + for i := range a.bucketsNew { + a.bucketsNew[i] = make(map[string]*knownAddress) } // Old addr buckets - a.addrOld = make([]map[string]*knownAddress, oldBucketCount) - for i := range a.addrOld { - a.addrOld[i] = make(map[string]*knownAddress) + a.bucketsOld = make([]map[string]*knownAddress, oldBucketCount) + for i := range a.bucketsOld { + a.bucketsOld[i] = make(map[string]*knownAddress) } } @@ -214,7 +214,7 @@ func (a *AddrBook) PickAddress(newBias int) *NetAddress { if pickFromOldBucket { var bucket map[string]*knownAddress for len(bucket) == 0 { - bucket = a.addrOld[a.rand.Intn(len(a.addrOld))] + bucket = a.bucketsOld[a.rand.Intn(len(a.bucketsOld))] } // pick a random ka from bucket. randIndex := a.rand.Intn(len(bucket)) @@ -228,7 +228,7 @@ func (a *AddrBook) PickAddress(newBias int) *NetAddress { } else { var bucket map[string]*knownAddress = nil for len(bucket) == 0 { - bucket = a.addrNew[a.rand.Intn(len(a.addrNew))] + bucket = a.bucketsNew[a.rand.Intn(len(a.bucketsNew))] } // pick a random ka from bucket. randIndex := a.rand.Intn(len(bucket)) @@ -380,7 +380,7 @@ func (a *AddrBook) loadFromFile(filePath string) bool { // Restore all the fields... // Restore the key a.key = aJSON.Key - // Restore .addrNew & .addrOld + // Restore .bucketsNew & .bucketsOld for _, ka := range aJSON.Addrs { for _, bucketIndex := range ka.Buckets { bucket := a.getBucket(ka.BucketType, bucketIndex) @@ -425,9 +425,9 @@ out: func (a *AddrBook) getBucket(bucketType byte, bucketIdx int) map[string]*knownAddress { switch bucketType { case bucketTypeNew: - return a.addrNew[bucketIdx] + return a.bucketsNew[bucketIdx] case bucketTypeOld: - return a.addrOld[bucketIdx] + return a.bucketsOld[bucketIdx] default: cmn.PanicSanity("Should not happen") return nil @@ -587,7 +587,7 @@ func (a *AddrBook) addAddress(addr, src *NetAddress) { // Make space in the new buckets by expiring the really bad entries. // If no bad entries are available we remove the oldest. func (a *AddrBook) expireNew(bucketIdx int) { - for addrStr, ka := range a.addrNew[bucketIdx] { + for addrStr, ka := range a.bucketsNew[bucketIdx] { // If an entry is bad, throw it away if ka.isBad() { a.Logger.Info(cmn.Fmt("expiring bad address %v", addrStr)) From ed95cc160a44a0949d11a52c0d853c80d111511d Mon Sep 17 00:00:00 2001 From: Ethan Buchman Date: Thu, 16 Nov 2017 02:31:47 +0000 Subject: [PATCH 04/16] p2p/addrbook: simplify PickAddress --- p2p/addrbook.go | 33 +++++++-------------------------- 1 file changed, 7 insertions(+), 26 deletions(-) diff --git a/p2p/addrbook.go b/p2p/addrbook.go index eeeeaf5ee..3cc385d1d 100644 --- a/p2p/addrbook.go +++ b/p2p/addrbook.go @@ -210,37 +210,18 @@ func (a *AddrBook) PickAddress(newBias int) *NetAddress { oldCorrelation := math.Sqrt(float64(a.nOld)) * (100.0 - float64(newBias)) newCorrelation := math.Sqrt(float64(a.nNew)) * float64(newBias) + // pick a random peer from a random bucket + var bucket map[string]*knownAddress pickFromOldBucket := (newCorrelation+oldCorrelation)*a.rand.Float64() < oldCorrelation - if pickFromOldBucket { - var bucket map[string]*knownAddress - for len(bucket) == 0 { + for len(bucket) == 0 { + if pickFromOldBucket { bucket = a.bucketsOld[a.rand.Intn(len(a.bucketsOld))] - } - // pick a random ka from bucket. - randIndex := a.rand.Intn(len(bucket)) - for _, ka := range bucket { - if randIndex == 0 { - return ka.Addr - } - randIndex-- - } - cmn.PanicSanity("Should not happen") - } else { - var bucket map[string]*knownAddress = nil - for len(bucket) == 0 { + } else { bucket = a.bucketsNew[a.rand.Intn(len(a.bucketsNew))] } - // pick a random ka from bucket. - randIndex := a.rand.Intn(len(bucket)) - for _, ka := range bucket { - if randIndex == 0 { - return ka.Addr - } - randIndex-- - } - cmn.PanicSanity("Should not happen") } - return nil + randIndex := a.rand.Intn(len(bucket)) + return bucket[randIndex].Addr } // MarkGood marks the peer as good and moves it into an "old" bucket. From 8c88cc017a1afc3d577643667355e21ca2df9551 Mon Sep 17 00:00:00 2001 From: Ethan Buchman Date: Thu, 16 Nov 2017 03:59:54 +0000 Subject: [PATCH 05/16] p2p/addrbook: addAddress returns error. more defensive PickAddress --- p2p/addrbook.go | 34 +++++++++++++++++++++++----------- 1 file changed, 23 insertions(+), 11 deletions(-) diff --git a/p2p/addrbook.go b/p2p/addrbook.go index 3cc385d1d..842fcfdee 100644 --- a/p2p/addrbook.go +++ b/p2p/addrbook.go @@ -7,6 +7,7 @@ package p2p import ( "encoding/binary" "encoding/json" + "fmt" "math" "math/rand" "net" @@ -168,11 +169,10 @@ func (a *AddrBook) OurAddresses() []*NetAddress { // AddAddress adds the given address as received from the given source. // NOTE: addr must not be nil -func (a *AddrBook) AddAddress(addr *NetAddress, src *NetAddress) { +func (a *AddrBook) AddAddress(addr *NetAddress, src *NetAddress) error { a.mtx.Lock() defer a.mtx.Unlock() - a.Logger.Info("Add address to book", "addr", addr, "src", src) - a.addAddress(addr, src) + return a.addAddress(addr, src) } // NeedMoreAddrs returns true if there are not have enough addresses in the book. @@ -213,6 +213,11 @@ func (a *AddrBook) PickAddress(newBias int) *NetAddress { // pick a random peer from a random bucket var bucket map[string]*knownAddress pickFromOldBucket := (newCorrelation+oldCorrelation)*a.rand.Float64() < oldCorrelation + if (pickFromOldBucket && a.nOld == 0) || + (!pickFromOldBucket && a.nNew == 0) { + return nil + } + // loop until we pick a random non-empty bucket for len(bucket) == 0 { if pickFromOldBucket { bucket = a.bucketsOld[a.rand.Intn(len(a.bucketsOld))] @@ -220,8 +225,15 @@ func (a *AddrBook) PickAddress(newBias int) *NetAddress { bucket = a.bucketsNew[a.rand.Intn(len(a.bucketsNew))] } } + // pick a random index and loop over the map to return that index randIndex := a.rand.Intn(len(bucket)) - return bucket[randIndex].Addr + for _, ka := range bucket { + if randIndex == 0 { + return ka.Addr + } + randIndex-- + } + return nil } // MarkGood marks the peer as good and moves it into an "old" bucket. @@ -529,14 +541,13 @@ func (a *AddrBook) pickOldest(bucketType byte, bucketIdx int) *knownAddress { return oldest } -func (a *AddrBook) addAddress(addr, src *NetAddress) { +func (a *AddrBook) addAddress(addr, src *NetAddress) error { if a.routabilityStrict && !addr.Routable() { - a.Logger.Error(cmn.Fmt("Cannot add non-routable address %v", addr)) - return + return fmt.Errorf("Cannot add non-routable address %v", addr) } if _, ok := a.ourAddrs[addr.String()]; ok { // Ignore our own listener address. - return + return fmt.Errorf("Cannot add ourselves with address %v", addr) } ka := a.addrLookup[addr.String()] @@ -544,16 +555,16 @@ func (a *AddrBook) addAddress(addr, src *NetAddress) { if ka != nil { // Already old. if ka.isOld() { - return + return nil } // Already in max new buckets. if len(ka.Buckets) == maxNewBucketsPerAddress { - return + return nil } // The more entries we have, the less likely we are to add more. factor := int32(2 * len(ka.Buckets)) if a.rand.Int31n(factor) != 0 { - return + return nil } } else { ka = newKnownAddress(addr, src) @@ -563,6 +574,7 @@ func (a *AddrBook) addAddress(addr, src *NetAddress) { a.addToNewBucket(ka, bucket) a.Logger.Info("Added new address", "address", addr, "total", a.size()) + return nil } // Make space in the new buckets by expiring the really bad entries. From 435eb6e2b3b4d4ba25faffd98c61e7b0dab155ea Mon Sep 17 00:00:00 2001 From: Ethan Buchman Date: Thu, 16 Nov 2017 04:00:59 +0000 Subject: [PATCH 06/16] p2p/addrbook: add non-terminating test --- p2p/addrbook_test.go | 38 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/p2p/addrbook_test.go b/p2p/addrbook_test.go index 9b83be180..6ebb34c34 100644 --- a/p2p/addrbook_test.go +++ b/p2p/addrbook_test.go @@ -23,6 +23,40 @@ func createTempFileName(prefix string) string { return fname } +func TestAddrBookPickAddress(t *testing.T) { + assert := assert.New(t) + fname := createTempFileName("addrbook_test") + + // 0 addresses + book := NewAddrBook(fname, true) + book.SetLogger(log.TestingLogger()) + assert.Zero(book.Size()) + + addr := book.PickAddress(50) + assert.Nil(addr, "expected no address") + + randAddrs := randNetAddressPairs(t, 1) + addrSrc := randAddrs[0] + book.AddAddress(addrSrc.addr, addrSrc.src) + + // pick an address when we only have new address + addr = book.PickAddress(0) + assert.NotNil(addr, "expected an address") + addr = book.PickAddress(50) + assert.NotNil(addr, "expected an address") + addr = book.PickAddress(100) + assert.NotNil(addr, "expected an address") + + // pick an address when we only have old address + book.MarkGood(addrSrc.addr) + addr = book.PickAddress(0) + assert.NotNil(addr, "expected an address") + addr = book.PickAddress(50) + assert.NotNil(addr, "expected an address") + addr = book.PickAddress(100) + assert.NotNil(addr, "expected an address") +} + func TestAddrBookSaveLoad(t *testing.T) { fname := createTempFileName("addrbook_test") @@ -106,6 +140,10 @@ func TestAddrBookPromoteToOld(t *testing.T) { if len(selection) > book.Size() { t.Errorf("selection could not be bigger than the book") } + + if book.Size() != 100 { + t.Errorf("Size is not 100. Got %v", book.Size()) + } } func TestAddrBookHandlesDuplicates(t *testing.T) { From 40e93a5f9eb75ba65e064f197e6e3d79800d30a8 Mon Sep 17 00:00:00 2001 From: Ethan Buchman Date: Thu, 16 Nov 2017 04:08:46 +0000 Subject: [PATCH 07/16] p2p/addrbook: fix addToOldBucket --- p2p/addrbook.go | 2 +- p2p/addrbook_test.go | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/p2p/addrbook.go b/p2p/addrbook.go index 842fcfdee..1101ffeef 100644 --- a/p2p/addrbook.go +++ b/p2p/addrbook.go @@ -475,7 +475,7 @@ func (a *AddrBook) addToOldBucket(ka *knownAddress, bucketIdx int) bool { } addrStr := ka.Addr.String() - bucket := a.getBucket(bucketTypeNew, bucketIdx) + bucket := a.getBucket(bucketTypeOld, bucketIdx) // Already exists? if _, ok := bucket[addrStr]; ok { diff --git a/p2p/addrbook_test.go b/p2p/addrbook_test.go index 6ebb34c34..419081d01 100644 --- a/p2p/addrbook_test.go +++ b/p2p/addrbook_test.go @@ -53,8 +53,10 @@ func TestAddrBookPickAddress(t *testing.T) { assert.NotNil(addr, "expected an address") addr = book.PickAddress(50) assert.NotNil(addr, "expected an address") + + // in this case, nNew==0 but we biased 100% to new, so we return nil addr = book.PickAddress(100) - assert.NotNil(addr, "expected an address") + assert.Nil(addr, "did not expected an address") } func TestAddrBookSaveLoad(t *testing.T) { From 8e044b0e6ddee561a7aed99d2346db5ba44efb69 Mon Sep 17 00:00:00 2001 From: Ethan Buchman Date: Thu, 16 Nov 2017 04:30:23 +0000 Subject: [PATCH 08/16] p2p/addrbook: some comments --- p2p/addrbook.go | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/p2p/addrbook.go b/p2p/addrbook.go index 1101ffeef..00b7ef759 100644 --- a/p2p/addrbook.go +++ b/p2p/addrbook.go @@ -237,6 +237,7 @@ func (a *AddrBook) PickAddress(newBias int) *NetAddress { } // MarkGood marks the peer as good and moves it into an "old" bucket. +// XXX: we never call this! func (a *AddrBook) MarkGood(addr *NetAddress) { a.mtx.Lock() defer a.mtx.Unlock() @@ -304,6 +305,7 @@ func (a *AddrBook) GetSelection() []*NetAddress { // Fisher-Yates shuffle the array. We only need to do the first // `numAddresses' since we are throwing the rest. + // XXX: What's the point of this if we already loop randomly through addrLookup ? for i := 0; i < numAddresses; i++ { // pick a number between current index and the end j := rand.Intn(len(allAddr)-i) + i @@ -400,17 +402,17 @@ func (a *AddrBook) Save() { func (a *AddrBook) saveRoutine() { defer a.wg.Done() - dumpAddressTicker := time.NewTicker(dumpAddressInterval) + saveFileTicker := time.NewTicker(dumpAddressInterval) out: for { select { - case <-dumpAddressTicker.C: + case <-saveFileTicker.C: a.saveToFile(a.filePath) case <-a.Quit: break out } } - dumpAddressTicker.Stop() + saveFileTicker.Stop() a.saveToFile(a.filePath) a.Logger.Info("Address handler done") } From be1a16a6016286584a7dce3ffff682f9beab7a29 Mon Sep 17 00:00:00 2001 From: Ethan Buchman Date: Thu, 16 Nov 2017 04:30:38 +0000 Subject: [PATCH 09/16] p2p/pex: simplify ensurePeers --- p2p/pex_reactor.go | 49 +++++++++++++++------------------------------- 1 file changed, 16 insertions(+), 33 deletions(-) diff --git a/p2p/pex_reactor.go b/p2p/pex_reactor.go index e2ccff424..da72bd535 100644 --- a/p2p/pex_reactor.go +++ b/p2p/pex_reactor.go @@ -240,43 +240,26 @@ func (r *PEXReactor) ensurePeers() { return } - toDial := make(map[string]*NetAddress) + // bias to prefer more vetted peers when we have fewer connections. + // not perfect, but somewhate ensures that we prioritize connecting to more-vetted + newBias := cmn.MinInt(numOutPeers, 8)*10 + 10 - // Try to pick numToDial addresses to dial. - for i := 0; i < numToDial; i++ { - // The purpose of newBias is to first prioritize old (more vetted) peers - // when we have few connections, but to allow for new (less vetted) peers - // if we already have many connections. This algorithm isn't perfect, but - // it somewhat ensures that we prioritize connecting to more-vetted - // peers. - newBias := cmn.MinInt(numOutPeers, 8)*10 + 10 - var picked *NetAddress - // Try to fetch a new peer 3 times. - // This caps the maximum number of tries to 3 * numToDial. - for j := 0; j < 3; j++ { - try := r.book.PickAddress(newBias) - if try == nil { - break - } - _, alreadySelected := toDial[try.IP.String()] - alreadyDialing := r.Switch.IsDialing(try) - alreadyConnected := r.Switch.Peers().Has(try.IP.String()) - if alreadySelected || alreadyDialing || alreadyConnected { - // r.Logger.Info("Cannot dial address", "addr", try, - // "alreadySelected", alreadySelected, - // "alreadyDialing", alreadyDialing, - // "alreadyConnected", alreadyConnected) - continue - } else { - r.Logger.Info("Will dial address", "addr", try) - picked = try - break - } + toDial := make(map[string]*NetAddress) + // Try maxAttempts times to pick numToDial addresses to dial + maxAttempts := numToDial * 3 + for i := 0; i < maxAttempts && len(toDial) < numToDial; i++ { + try := r.book.PickAddress(newBias) + if try == nil { + continue } - if picked == nil { + _, alreadySelected := toDial[try.IP.String()] + alreadyDialing := r.Switch.IsDialing(try) + alreadyConnected := r.Switch.Peers().Has(try.IP.String()) + if alreadySelected || alreadyDialing || alreadyConnected { continue } - toDial[picked.IP.String()] = picked + r.Logger.Info("Will dial address", "addr", try) + toDial[try.IP.String()] = try } // Dial picked addresses From feb3230160fe4ecd40cd2bbb24fed5345db2305c Mon Sep 17 00:00:00 2001 From: Ethan Buchman Date: Thu, 16 Nov 2017 04:43:07 +0000 Subject: [PATCH 10/16] some comments --- p2p/addrbook.go | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/p2p/addrbook.go b/p2p/addrbook.go index 00b7ef759..8570dcf7a 100644 --- a/p2p/addrbook.go +++ b/p2p/addrbook.go @@ -684,8 +684,8 @@ func (a *AddrBook) calcOldBucket(addr *NetAddress) int { } // Return a string representing the network group of this address. -// This is the /16 for IPv6, the /32 (/36 for he.net) for IPv6, the string -// "local" for a local address and the string "unroutable for an unroutable +// This is the /16 for IPv4, the /32 (/36 for he.net) for IPv6, the string +// "local" for a local address and the string "unroutable" for an unroutable // address. func (a *AddrBook) groupKey(na *NetAddress) string { if a.routabilityStrict && na.Local() { @@ -811,8 +811,8 @@ func (ka *knownAddress) removeBucketRef(bucketIdx int) int { } /* - An address is bad if the address in question has not been tried in the last - minute and meets one of the following criteria: + An address is bad if the address in question is a New address, has not been tried in the last + minute, and meets one of the following criteria: 1) It claims to be from the future 2) It hasn't been seen in over a month @@ -821,8 +821,15 @@ func (ka *knownAddress) removeBucketRef(bucketIdx int) int { All addresses that meet these criteria are assumed to be worthless and not worth keeping hold of. + + XXX: so a good peer needs us to call MarkGood before the conditions above are reached! */ func (ka *knownAddress) isBad() bool { + // Is Old --> good + if ka.BucketType == bucketTypeOld { + return false + } + // Has been attempted in the last minute --> good if ka.LastAttempt.Before(time.Now().Add(-1 * time.Minute)) { return false @@ -830,6 +837,7 @@ func (ka *knownAddress) isBad() bool { // Too old? // XXX: does this mean if we've kept a connection up for this long we'll disconnect?! + // and shouldn't it be .Before ? if ka.LastAttempt.After(time.Now().Add(-1 * numMissingDays * time.Hour * 24)) { return true } @@ -840,6 +848,7 @@ func (ka *knownAddress) isBad() bool { } // Hasn't succeeded in too long? + // XXX: does this mean if we've kept a connection up for this long we'll disconnect?! if ka.LastSuccess.Before(time.Now().Add(-1*minBadDays*time.Hour*24)) && ka.Attempts >= maxFailures { return true From 031e10133c094948eb224ae9b15b43b9093e130b Mon Sep 17 00:00:00 2001 From: Emmanuel Odeke Date: Sat, 18 Nov 2017 22:17:53 -0700 Subject: [PATCH 11/16] p2p: make Switch.DialSeeds use a new PRNG per call Fixes https://github.com/tendermint/tendermint/issues/875 Ensure that every DialSeeds call uses a new PRNG seeded from tendermint/tmlibs/common.RandInt which internally uses crypto/rand to seed its source. --- p2p/switch.go | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/p2p/switch.go b/p2p/switch.go index b56e84a81..dcdc9c1d6 100644 --- a/p2p/switch.go +++ b/p2p/switch.go @@ -295,7 +295,6 @@ func (sw *Switch) startInitPeer(peer *peer) { // DialSeeds dials a list of seeds asynchronously in random order. func (sw *Switch) DialSeeds(addrBook *AddrBook, seeds []string) error { - netAddrs, err := NewNetAddressStrings(seeds) if err != nil { return err @@ -315,11 +314,15 @@ func (sw *Switch) DialSeeds(addrBook *AddrBook, seeds []string) error { addrBook.Save() } + // Ensure we have a completely undeterministic PRNG. cmd.RandInt64() draws + // from a seed that's initialized with OS entropy on process start. + rng := rand.New(rand.NewSource(cmn.RandInt64())) + // permute the list, dial them in random order. - perm := rand.Perm(len(netAddrs)) + perm := rng.Perm(len(netAddrs)) for i := 0; i < len(perm); i++ { go func(i int) { - time.Sleep(time.Duration(rand.Int63n(3000)) * time.Millisecond) + time.Sleep(time.Duration(rng.Int63n(3000)) * time.Millisecond) j := perm[i] sw.dialSeed(netAddrs[j]) }(i) From 882c25f2923c57fb140c2288f74972b49ff5e890 Mon Sep 17 00:00:00 2001 From: "A. F. Dudley" Date: Tue, 21 Nov 2017 10:11:48 -0500 Subject: [PATCH 12/16] Update getting-started.rst to fix broken link fixes broken link to introduction.html --- docs/getting-started.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/getting-started.rst b/docs/getting-started.rst index a9a391b0e..26f6b7897 100644 --- a/docs/getting-started.rst +++ b/docs/getting-started.rst @@ -5,7 +5,7 @@ As a general purpose blockchain engine, Tendermint is agnostic to the application you want to run. So, to run a complete blockchain that does something useful, you must start two programs: one is Tendermint Core, the other is your application, which can be written in any programming -language. Recall from `the intro to ABCI `__ that +language. Recall from `the intro to ABCI `__ that Tendermint Core handles all the p2p and consensus stuff, and just forwards transactions to the application when they need to be validated, or when they're ready to be committed to a block. From c4b695f78da152d7f4d1fbc515c13fcf457c2d75 Mon Sep 17 00:00:00 2001 From: Ethan Buchman Date: Mon, 20 Nov 2017 19:30:05 +0000 Subject: [PATCH 13/16] minor fixes from review --- p2p/addrbook.go | 7 ++++++- p2p/addrbook_test.go | 5 ++--- p2p/pex_reactor.go | 11 +++++++---- 3 files changed, 15 insertions(+), 8 deletions(-) diff --git a/p2p/addrbook.go b/p2p/addrbook.go index 8570dcf7a..0b3301060 100644 --- a/p2p/addrbook.go +++ b/p2p/addrbook.go @@ -191,7 +191,12 @@ func (a *AddrBook) size() int { return a.nNew + a.nOld } -// PickAddress picks an address to connect to with new/old bias. +// PickAddress picks an address to connect to. +// The address is picked randomly from an old or new bucket according +// to the newBias argument, which must be between [0, 100] (or else is truncated to that range) +// and determines how biased we are to pick an address from a new bucket. +// PickAddress returns nil if the AddrBook is empty or if we try to pick +// from an empty bucket. func (a *AddrBook) PickAddress(newBias int) *NetAddress { a.mtx.Lock() defer a.mtx.Unlock() diff --git a/p2p/addrbook_test.go b/p2p/addrbook_test.go index 419081d01..d84c008ed 100644 --- a/p2p/addrbook_test.go +++ b/p2p/addrbook_test.go @@ -112,6 +112,7 @@ func TestAddrBookLookup(t *testing.T) { } func TestAddrBookPromoteToOld(t *testing.T) { + assert := assert.New(t) fname := createTempFileName("addrbook_test") randAddrs := randNetAddressPairs(t, 100) @@ -143,9 +144,7 @@ func TestAddrBookPromoteToOld(t *testing.T) { t.Errorf("selection could not be bigger than the book") } - if book.Size() != 100 { - t.Errorf("Size is not 100. Got %v", book.Size()) - } + assert.Equal(book.Size(), 100, "expecting book size to be 100") } func TestAddrBookHandlesDuplicates(t *testing.T) { diff --git a/p2p/pex_reactor.go b/p2p/pex_reactor.go index da72bd535..fd70198f4 100644 --- a/p2p/pex_reactor.go +++ b/p2p/pex_reactor.go @@ -252,10 +252,13 @@ func (r *PEXReactor) ensurePeers() { if try == nil { continue } - _, alreadySelected := toDial[try.IP.String()] - alreadyDialing := r.Switch.IsDialing(try) - alreadyConnected := r.Switch.Peers().Has(try.IP.String()) - if alreadySelected || alreadyDialing || alreadyConnected { + if _, selected := toDial[try.IP.String()]; selected { + continue + } + if dialling := r.Switch.IsDialing(try); dialling { + continue + } + if connected := r.Switch.Peers().Has(try.IP.String()); connected { continue } r.Logger.Info("Will dial address", "addr", try) From e110f70b5cd26a78565c7dae91ac896d4b064dbe Mon Sep 17 00:00:00 2001 From: Petabyte Storage Date: Wed, 22 Nov 2017 07:34:10 -0800 Subject: [PATCH 14/16] update glide.yaml versions with go-wire at develop branch --- glide.lock | 38 +++++++++++++++++++------------------- glide.yaml | 2 +- 2 files changed, 20 insertions(+), 20 deletions(-) diff --git a/glide.lock b/glide.lock index 13127b076..fdea86519 100644 --- a/glide.lock +++ b/glide.lock @@ -1,5 +1,5 @@ -hash: 0f9ba99fd411afaaf90993037b0067c5f9f873554f407a6ae9afa0e2548343c5 -updated: 2017-10-27T22:34:38.187149434-04:00 +hash: 223d8e42a118e7861cb673ea58a035e99d3a98c94e4b71fb52998d320f9c3b49 +updated: 2017-11-22T07:33:50.996598926-08:00 imports: - name: github.com/btcsuite/btcd version: 8cea3866d0f7fb12d567a20744942c0d078c7d15 @@ -10,7 +10,7 @@ imports: - name: github.com/fsnotify/fsnotify version: 4da3e2cfbabc9f751898f250b49f2439785783a1 - name: github.com/go-kit/kit - version: e2b298466b32c7cd5579a9b9b07e968fc9d9452c + version: e3b2152e0063c5f05efea89ecbe297852af2a92d subpackages: - log - log/level @@ -24,13 +24,13 @@ imports: - name: github.com/go-playground/universal-translator version: 71201497bace774495daed26a3874fd339e0b538 - name: github.com/go-stack/stack - version: 817915b46b97fd7bb80e8ab6b69f01a53ac3eebf + version: 259ab82a6cad3992b4e21ff5cac294ccb06474bc - name: github.com/gogo/protobuf version: 342cbe0a04158f6dcb03ca0079991a51a4248c02 subpackages: - proto - name: github.com/golang/protobuf - version: 1643683e1b54a9e88ad26d98f81400c8c9d9f4f9 + version: 1e59b77b52bf8e4b449a57e6f79f21226d571845 subpackages: - proto - ptypes @@ -59,7 +59,7 @@ imports: - name: github.com/kr/logfmt version: b84e30acd515aadc4b783ad4ff83aff3299bdfe0 - name: github.com/magiconair/properties - version: 8d7837e64d3c1ee4e54a880c5a920ab4316fc90a + version: 49d762b9817ba1c2e9d0c69183c2b4a8b8f1d934 - name: github.com/mitchellh/mapstructure version: 06020f85339e21b2478f756a78e295255ffa4d6a - name: github.com/pelletier/go-toml @@ -69,7 +69,7 @@ imports: - name: github.com/rcrowley/go-metrics version: 1f30fe9094a513ce4c700b9a54458bbb0c96996c - name: github.com/spf13/afero - version: 5660eeed305fe5f69c8fc6cf899132a459a97064 + version: 8d919cbe7e2627e417f3e45c3c0e489a5b7e2536 subpackages: - mem - name: github.com/spf13/cast @@ -79,11 +79,11 @@ imports: - name: github.com/spf13/jwalterweatherman version: 12bd96e66386c1960ab0f74ced1362f66f552f7b - name: github.com/spf13/pflag - version: 97afa5e7ca8a08a383cb259e06636b5e2cc7897f + version: 4c012f6dcd9546820e378d0bdda4d8fc772cdfea - name: github.com/spf13/viper version: 25b30aa063fc18e48662b86996252eabdcf2f0c7 - name: github.com/syndtr/goleveldb - version: b89cc31ef7977104127d34c1bd31ebd1a9db2199 + version: adf24ef3f94bd13ec4163060b21a5678f22b429b subpackages: - leveldb - leveldb/cache @@ -98,7 +98,7 @@ imports: - leveldb/table - leveldb/util - name: github.com/tendermint/abci - version: dc33aad9b4e514a2322725ef68f27f72d955c537 + version: 76ef8a0697c6179220a74c479b36c27a5b53008a subpackages: - client - example/counter @@ -113,10 +113,11 @@ imports: - name: github.com/tendermint/go-crypto version: dd20358a264c772b4a83e477b0cfce4c88a7001d - name: github.com/tendermint/go-wire - version: 2baffcb6b690057568bc90ef1d457efb150b979a + version: 7d50b38b3815efe313728de77e2995c8813ce13f subpackages: - data - data/base58 + - nowriter/tmencoding - name: github.com/tendermint/iavl version: 594cc0c062a7174475f0ab654384038d77067917 subpackages: @@ -130,7 +131,6 @@ imports: - clist - common - db - - events - flowrate - log - merkle @@ -138,7 +138,7 @@ imports: - pubsub/query - test - name: golang.org/x/crypto - version: 2509b142fb2b797aa7587dad548f113b2c0f20ce + version: 9f005a07e0d31d45e6656d241bb5c0f2efd4bc94 subpackages: - curve25519 - nacl/box @@ -149,7 +149,7 @@ imports: - ripemd160 - salsa20/salsa - name: golang.org/x/net - version: c73622c77280266305273cb545f54516ced95b93 + version: 9dfe39835686865bff950a07b394c12a98ddc811 subpackages: - context - http2 @@ -159,18 +159,18 @@ imports: - lex/httplex - trace - name: golang.org/x/sys - version: b98136db334ff9cb24f28a68e3be3cb6608f7630 + version: 82aafbf43bf885069dc71b7e7c2f9d7a614d47da subpackages: - unix - name: golang.org/x/text - version: 6eab0e8f74e86c598ec3b6fad4888e0c11482d48 + version: 88f656faf3f37f690df1a32515b479415e1a6769 subpackages: - secure/bidirule - transform - unicode/bidi - unicode/norm - name: google.golang.org/genproto - version: f676e0f3ac6395ff1a529ae59a6670878a8371a6 + version: 891aceb7c239e72692819142dfca057bdcbfcb96 subpackages: - googleapis/rpc/status - name: google.golang.org/grpc @@ -193,9 +193,9 @@ imports: - tap - transport - name: gopkg.in/go-playground/validator.v9 - version: 1304298bf10d085adec514b076772a79c9cadb6b + version: 61caf9d3038e1af346dbf5c2e16f6678e1548364 - name: gopkg.in/yaml.v2 - version: eb3733d160e74a9c7e442f435eb3bea458e1d19f + version: 287cf08546ab5e7e37d55a84f7ed3fd1db036de5 testImports: - name: github.com/davecgh/go-spew version: 04cdfd42973bb9c8589fd6a731800cf222fde1a9 diff --git a/glide.yaml b/glide.yaml index a305f0b79..ddce26cc6 100644 --- a/glide.yaml +++ b/glide.yaml @@ -26,7 +26,7 @@ import: - package: github.com/tendermint/go-crypto version: ~0.4.1 - package: github.com/tendermint/go-wire - version: ~0.7.1 + version: develop subpackages: - data - package: github.com/tendermint/iavl From 969b34057bb311252aa484d86c40d8fa26deef2b Mon Sep 17 00:00:00 2001 From: Zach Ramsay Date: Wed, 22 Nov 2017 17:22:53 +0000 Subject: [PATCH 15/16] remove unused file --- INSTALL.md | 1 - 1 file changed, 1 deletion(-) delete mode 100644 INSTALL.md diff --git a/INSTALL.md b/INSTALL.md deleted file mode 100644 index 35b5ffec0..000000000 --- a/INSTALL.md +++ /dev/null @@ -1 +0,0 @@ -The installation guide has moved to the [docs directory](docs/guides/install-from-source.md) in order to easily be rendered by the website. Please update your links accordingly. From e8459875037f1274d0dbb6e2056a32943186a924 Mon Sep 17 00:00:00 2001 From: Ethan Buchman Date: Wed, 22 Nov 2017 20:20:53 +0000 Subject: [PATCH 16/16] p2p: disable trustmetric test while being fixed --- p2p/trust/{trustmetric_test.go => trustmetric_test.go_} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename p2p/trust/{trustmetric_test.go => trustmetric_test.go_} (100%) diff --git a/p2p/trust/trustmetric_test.go b/p2p/trust/trustmetric_test.go_ similarity index 100% rename from p2p/trust/trustmetric_test.go rename to p2p/trust/trustmetric_test.go_