Merge pull request #1528 from tendermint/release/v0.19.2

Release/v0.19.2

CHANGELOG.md

@@ -24,6 +24,28 @@ BUG FIXES:
 - Graceful handling/recovery for apps that have non-determinism or fail to halt
 - Graceful handling/recovery for violations of safety, or liveness
 
+## 0.19.2 (April 30th, 2018)
+
+FEATURES:
+
+- [p2p] Allow peers with different Minor versions to connect
+- [rpc] `/net_info` includes `n_peers`
+
+IMPROVEMENTS:
+
+- [p2p] Various code comments, cleanup, error types
+- [p2p] Change some Error logs to Debug
+
+BUG FIXES:
+
+- [p2p] Fix reconnect to persistent peer when first dial fails
+- [p2p] Validate NodeInfo.ListenAddr
+- [p2p] Only allow (MaxNumPeers - MaxNumOutboundPeers) inbound peers
+- [p2p/pex] Limit max msg size to 64kB
+- [p2p] Fix panic when pex=false
+- [p2p] Allow multiple IPs per ID in AddrBook
+- [p2p] Fix before/after bugs in addrbook isBad()
+
 ## 0.19.1 (April 27th, 2018)
 
 Note this release includes some small breaking changes in the RPC and one in the

docs/specification/new-spec/abci.md

@@ -5,15 +5,47 @@ and an application (the actual state machine).
 
 The ABCI message types are defined in a [protobuf
 file](https://github.com/tendermint/abci/blob/master/types/types.proto).
+
 For full details on the ABCI message types and protocol, see the [ABCI
 specificaiton](https://github.com/tendermint/abci/blob/master/specification.rst).
+Be sure to read the specification if you're trying to build an ABCI app!
+
 For additional details on server implementation, see the [ABCI
 readme](https://github.com/tendermint/abci#implementation).
 
 Here we provide some more details around the use of ABCI by Tendermint and
 clarify common "gotchas".
 
-## Validator Updates
+## ABCI connections
+
+Tendermint opens 3 ABCI connections to the app: one for Consensus, one for
+Mempool, one for Queries.
+
+## Async vs Sync
+
+The main ABCI server (ie. non-GRPC) provides ordered asynchronous messages.
+This is useful for DeliverTx and CheckTx, since it allows Tendermint to forward
+transactions to the app before it's finished processing previous ones.
+
+Thus, DeliverTx and CheckTx messages are sent asycnhronously, while all other
+messages are sent synchronously.
+
+## CheckTx and Commit
+
+It is typical to hold three distinct states in an ABCI app: CheckTxState, DeliverTxState,
+QueryState. The QueryState contains the latest committed state for a block.
+The CheckTxState and DeliverTxState may be updated concurrently with one another.
+Before Commit is called, Tendermint locks and flushes the mempool so that no new changes will happen
+to CheckTxState. When Commit completes, it unlocks the mempool.
+
+Thus, during Commit, it is safe to reset the QueryState and the CheckTxState to the latest DeliverTxState
+(ie. the new state from executing all the txs in the block).
+
+Note, however, that it is not possible to send transactions to Tendermint during Commit - if your app
+tries to send a `/broadcast_tx` to Tendermint during Commit, it will deadlock.
+
+
+## EndBlock Validator Updates
 
 Updates to the Tendermint validator set can be made by returning `Validator`
 objects in the `ResponseBeginBlock`:
@@ -67,3 +99,70 @@ using the following paths, with no additional data:
 
 If either of these queries return a non-zero ABCI code, Tendermint will refuse
 to connect to the peer.
+
+## Info and the Handshake/Replay
+
+On startup, Tendermint calls Info on the Query connection to get the latest
+committed state of the app. The app MUST return information consistent with the
+last block it succesfully completed Commit for.
+
+If the app succesfully committed block H but not H+1, then `last_block_height =
+H` and `last_block_app_hash = <hash returned by Commit for block H>`. If the app
+failed during the Commit of block H, then `last_block_height = H-1` and
+`last_block_app_hash = <hash returned by Commit for block H-1, which is the hash
+in the header of block H>`.
+
+We now distinguish three heights, and describe how Tendermint syncs itself with
+the app.
+
+```
+storeBlockHeight = height of the last block Tendermint saw a commit for
+stateBlockHeight = height of the last block for which Tendermint completed all
+    block processing and saved all ABCI results to disk
+appBlockHeight = height of the last block for which ABCI app succesfully
+    completely Commit
+
+```
+
+Note we always have `storeBlockHeight >= stateBlockHeight` and `storeBlockHeight >= appBlockHeight`
+Note also we never call Commit on an ABCI app twice for the same height.
+
+The procedure is as follows.
+
+First, some simeple start conditions:
+
+If `appBlockHeight == 0`, then call InitChain.
+
+If `storeBlockHeight == 0`, we're done.
+
+Now, some sanity checks:
+
+If `storeBlockHeight < appBlockHeight`, error
+If `storeBlockHeight < stateBlockHeight`, panic
+If `storeBlockHeight > stateBlockHeight+1`, panic
+
+Now, the meat:
+
+If `storeBlockHeight == stateBlockHeight && appBlockHeight < storeBlockHeight`,
+	replay all blocks in full from `appBlockHeight` to `storeBlockHeight`.
+	This happens if we completed processing the block, but the app forgot its height.
+
+If `storeBlockHeight == stateBlockHeight && appBlockHeight == storeBlockHeight`, we're done
+	This happens if we crashed at an opportune spot.
+
+If `storeBlockHeight == stateBlockHeight+1`
+	This happens if we started processing the block but didn't finish.
+
+	If `appBlockHeight < stateBlockHeight`
+		replay all blocks in full from `appBlockHeight` to `storeBlockHeight-1`,
+		and replay the block at `storeBlockHeight` using the WAL.
+	This happens if the app forgot the last block it committed.
+
+	If `appBlockHeight == stateBlockHeight`,
+		replay the last block (storeBlockHeight) in full.
+	This happens if we crashed before the app finished Commit
+
+	If appBlockHeight == storeBlockHeight {
+		update the state using the saved ABCI responses but dont run the block against the real app.
+	This happens if we crashed after the app finished Commit but before Tendermint saved the state.
+

docs/specification/new-spec/p2p/node.md

@@ -12,7 +12,7 @@ Seeds should operate full nodes with the PEX reactor in a "crawler" mode
 that continuously explores to validate the availability of peers.
 
 Seeds should only respond with some top percentile of the best peers it knows about.
-See [reputation](TODO) for details on peer quality.
+See [the peer-exchange docs](/docs/specification/new-spec/reactors/pex/pex.md)for details on peer quality.
 
 ## New Full Node
 

docs/specification/new-spec/p2p/peer.md

@@ -2,24 +2,23 @@
 
 This document explains how Tendermint Peers are identified and how they connect to one another.
 
-For details on peer discovery, see the [peer exchange (PEX) reactor doc](pex.md).
+For details on peer discovery, see the [peer exchange (PEX) reactor doc](/docs/specification/new-spec/reactors/pex/pex.md).
 
 ## Peer Identity
 
 Tendermint peers are expected to maintain long-term persistent identities in the form of a public key.
 Each peer has an ID defined as `peer.ID == peer.PubKey.Address()`, where `Address` uses the scheme defined in go-crypto.
 
-A single peer ID can have multiple IP addresses associated with it.
-TODO: define how to deal with this.
+A single peer ID can have multiple IP addresses associated with it, but a node
+will only ever connect to one at a time.
 
 When attempting to connect to a peer, we use the PeerURL: `<ID>@<IP>:<PORT>`.
 We will attempt to connect to the peer at IP:PORT, and verify,
 via authenticated encryption, that it is in possession of the private key
 corresponding to `<ID>`. This prevents man-in-the-middle attacks on the peer layer.
 
-Peers can also be connected to without specifying an ID, ie. just `<IP>:<PORT>`.
-In this case, the peer must be authenticated out-of-band of Tendermint,
-for instance via VPN.
+If `auth_enc = false`, peers can use an arbitrary ID, but they must always use
+one. Authentication can then happen out-of-band of Tendermint, for instance via VPN.
 
 ## Connections
 
@@ -84,12 +83,13 @@ The Tendermint Version Handshake allows the peers to exchange their NodeInfo:
 ```golang
 type NodeInfo struct {
   ID         p2p.ID
-  Moniker    string
-  Network    string
-  RemoteAddr string
   ListenAddr string
+
+  Network    string
   Version    string
   Channels   []int8
+
+  Moniker    string
   Other      []string
 }
 ```
@@ -98,9 +98,10 @@ The connection is disconnected if:
 - `peer.NodeInfo.ID` is not equal `peerConn.ID`
 - `peer.NodeInfo.Version` is not formatted as `X.X.X` where X are integers known as Major, Minor, and Revision
 - `peer.NodeInfo.Version` Major is not the same as ours
-- `peer.NodeInfo.Version` Minor is not the same as ours
 - `peer.NodeInfo.Network` is not the same as ours
 - `peer.Channels` does not intersect with our known Channels.
+- `peer.NodeInfo.ListenAddr` is malformed or is a DNS host that cannot be
+  resolved
 
 
 At this point, if we have not disconnected, the peer is valid.

docs/specification/new-spec/reactors/pex/pex.md

@@ -7,7 +7,8 @@ to good peers and to gossip peers to others.
 ## Peer Types
 
 Certain peers are special in that they are specified by the user as `persistent`,
-which means we auto-redial them if the connection fails.
+which means we auto-redial them if the connection fails, or if we fail to dial
+them.
 Some peers can be marked as `private`, which means
 we will not put them in the address book or gossip them to others.
 
@@ -19,22 +20,37 @@ Peer discovery begins with a list of seeds.
 When we have no peers, or have been unable to find enough peers from existing ones,
 we dial a randomly selected seed to get a list of peers to dial.
 
-So long as we have less than `MaxPeers`, we periodically request additional peers
+On startup, we will also immediately dial the given list of `persistent_peers`,
+and will attempt to maintain persistent connections with them. If the connections die, or we fail to dial,
+we will redial every 5s for a few minutes, then switch to an exponential backoff schedule,
+and after about a day of trying, stop dialing the peer.
+
+So long as we have less than `MinNumOutboundPeers`, we periodically request additional peers
 from each of our own. If sufficient time goes by and we still can't find enough peers,
 we try the seeds again.
 
+## Listening
+
+Peers listen on a configurable ListenAddr that they self-report in their
+NodeInfo during handshakes with other peers. Peers accept up to (MaxNumPeers -
+MinNumOutboundPeers) incoming peers.
+
 ## Address Book
 
 Peers are tracked via their ID (their PubKey.Address()).
-For each ID, the address book keeps the most recent IP:PORT.
 Peers are added to the address book from the PEX when they first connect to us or
 when we hear about them from other peers.
 
 The address book is arranged in sets of buckets, and distinguishes between
 vetted (old) and unvetted (new) peers. It keeps different sets of buckets for vetted and
-unvetted peers. Buckets provide randomization over peer selection.
+unvetted peers. Buckets provide randomization over peer selection. Peers are put
+in buckets according to their IP groups.
 
-A vetted peer can only be in one bucket. An unvetted peer can be in multiple buckets.
+A vetted peer can only be in one bucket. An unvetted peer can be in multiple buckets, and
+each instance of the peer can have a different IP:PORT.
+
+If we're trying to add a new peer but there's no space in its bucket, we'll
+remove the worst peer from that bucket to make room.
 
 ## Vetting
 
@@ -68,6 +84,8 @@ Connection attempts are made with exponential backoff (plus jitter). Because
 the selection process happens every `ensurePeersPeriod`, we might not end up
 dialing a peer for much longer than the backoff duration.
 
+If we fail to connect to the peer after 16 tries (with exponential backoff), we remove from address book completely.
+
 ## Select Peers to Exchange
 
 When we’re asked for peers, we select them as follows:
@@ -86,9 +104,9 @@ Note that the bad behaviour may be detected outside the PEX reactor itself
 (for instance, in the mconnection, or another reactor), but it must be communicated to the PEX reactor
 so it can remove and mark the peer.
 
-In the PEX, if a peer sends us unsolicited lists of peers,
-or if the peer sends too many requests for more peers in a given amount of time,
-we Disconnect and Mark.
+In the PEX, if a peer sends us an unsolicited list of peers,
+or if the peer sends a request too soon after another one,
+we Disconnect and MarkBad.
 
 ## Trust Metric
 

networks/local/localnode/Dockerfile

@@ -9,7 +9,7 @@ VOLUME [ /tendermint ]
 WORKDIR /tendermint
 EXPOSE 46656 46657
 ENTRYPOINT ["/usr/bin/wrapper.sh"]
-CMD ["node", "--proxy_app dummy"]
+CMD ["node", "--proxy_app", "dummy"]
 STOPSIGNAL SIGTERM
 
 COPY wrapper.sh /usr/bin/wrapper.sh

node/node.go

@@ -21,7 +21,6 @@ import (
 	mempl "github.com/tendermint/tendermint/mempool"
 	"github.com/tendermint/tendermint/p2p"
 	"github.com/tendermint/tendermint/p2p/pex"
-	"github.com/tendermint/tendermint/p2p/trust"
 	"github.com/tendermint/tendermint/proxy"
 	rpccore "github.com/tendermint/tendermint/rpc/core"
 	ctypes "github.com/tendermint/tendermint/rpc/core/types"
@@ -99,9 +98,8 @@ type Node struct {
 	privValidator types.PrivValidator // local node's validator key
 
 	// network
-	sw               *p2p.Switch             // p2p connections
-	addrBook         pex.AddrBook            // known peers
-	trustMetricStore *trust.TrustMetricStore // trust metrics for all peers
+	sw       *p2p.Switch  // p2p connections
+	addrBook pex.AddrBook // known peers
 
 	// services
 	eventBus         *types.EventBus // pub/sub for services
@@ -262,20 +260,24 @@ func NewNode(config *cfg.Config,
 	sw.AddReactor("EVIDENCE", evidenceReactor)
 
 	// Optionally, start the pex reactor
-	var addrBook pex.AddrBook
-	var trustMetricStore *trust.TrustMetricStore
+	//
+	// TODO:
+	//
+	// We need to set Seeds and PersistentPeers on the switch,
+	// since it needs to be able to use these (and their DNS names)
+	// even if the PEX is off. We can include the DNS name in the NetAddress,
+	// but it would still be nice to have a clear list of the current "PersistentPeers"
+	// somewhere that we can return with net_info.
+	//
+	// Let's assume we always have IDs ... and we just dont authenticate them
+	// if auth_enc=false.
+	//
+	// If PEX is on, it should handle dialing the seeds. Otherwise the switch does it.
+	// Note we currently use the addrBook regardless at least for AddOurAddress
+	addrBook := pex.NewAddrBook(config.P2P.AddrBookFile(), config.P2P.AddrBookStrict)
+	addrBook.SetLogger(p2pLogger.With("book", config.P2P.AddrBookFile()))
 	if config.P2P.PexReactor {
-		addrBook = pex.NewAddrBook(config.P2P.AddrBookFile(), config.P2P.AddrBookStrict)
-		addrBook.SetLogger(p2pLogger.With("book", config.P2P.AddrBookFile()))
-
-		// Get the trust metric history data
-		trustHistoryDB, err := dbProvider(&DBContext{"trusthistory", config})
-		if err != nil {
-			return nil, err
-		}
-		trustMetricStore = trust.NewTrustMetricStore(trustHistoryDB, trust.DefaultConfig())
-		trustMetricStore.SetLogger(p2pLogger)
-
+		// TODO persistent peers ? so we can have their DNS addrs saved
 		pexReactor := pex.NewPEXReactor(addrBook,
 			&pex.PEXReactorConfig{
 				Seeds:          cmn.SplitAndTrim(config.P2P.Seeds, ",", " "),
@@ -355,9 +357,8 @@ func NewNode(config *cfg.Config,
 		genesisDoc:    genDoc,
 		privValidator: privValidator,
 
-		sw:               sw,
-		addrBook:         addrBook,
-		trustMetricStore: trustMetricStore,
+		sw:       sw,
+		addrBook: addrBook,
 
 		stateDB:          stateDB,
 		blockStore:       blockStore,

p2p/CHANGELOG.md

@@ -1,78 +0,0 @@
-# Changelog
-
-## 0.5.0 (April 21, 2017)
-
-BREAKING CHANGES: 
-
-- Remove or unexport methods from FuzzedConnection: Active, Mode, ProbDropRW, ProbDropConn, ProbSleep, MaxDelayMilliseconds, Fuzz
-- switch.AddPeerWithConnection is unexported and replaced by switch.AddPeer
-- switch.DialPeerWithAddress takes a bool, setting the peer as persistent or not
-
-FEATURES:
-
-- Persistent peers: any peer considered a "seed" will be reconnected to when the connection is dropped
-
-
-IMPROVEMENTS:
-
-- Many more tests and comments
-- Refactor configurations for less dependence on go-config. Introduces new structs PeerConfig, MConnConfig, FuzzConnConfig
-- New methods on peer: CloseConn, HandshakeTimeout, IsPersistent, Addr, PubKey
-- NewNetAddress supports a testing mode where the address defaults to 0.0.0.0:0
-
-
-## 0.4.0 (March 6, 2017)
-
-BREAKING CHANGES: 
-
-- DialSeeds now takes an AddrBook and returns an error: `DialSeeds(*AddrBook, []string) error`
-- NewNetAddressString now returns an error: `NewNetAddressString(string) (*NetAddress, error)`
-
-FEATURES:
-
-- `NewNetAddressStrings([]string) ([]*NetAddress, error)`
-- `AddrBook.Save()`
-
-IMPROVEMENTS:
-
-- PexReactor responsible for starting and stopping the AddrBook
-
-BUG FIXES:
-
-- DialSeeds returns an error instead of panicking on bad addresses
-
-## 0.3.5 (January 12, 2017)
-
-FEATURES
-
-- Toggle strict routability in the AddrBook 
-
-BUG FIXES
-
-- Close filtered out connections
-- Fixes for MakeConnectedSwitches and Connect2Switches
-
-## 0.3.4 (August 10, 2016)
-
-FEATURES:
-
-- Optionally filter connections by address or public key
-
-## 0.3.3 (May 12, 2016)
-
-FEATURES:
-
-- FuzzConn
-
-## 0.3.2 (March 12, 2016)
-
-IMPROVEMENTS:
-
-- Memory optimizations
-
-## 0.3.1 ()
-
-FEATURES: 
-
-- Configurable parameters
-

p2p/errors.go

@@ -18,3 +18,31 @@ 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)
 }
+
+//-------------------------------------------------------------------
+
+type ErrNetAddressNoID struct {
+	Addr string
+}
+
+func (e ErrNetAddressNoID) Error() string {
+	return fmt.Sprintf("Address (%s) does not contain ID", e.Addr)
+}
+
+type ErrNetAddressInvalid struct {
+	Addr string
+	Err  error
+}
+
+func (e ErrNetAddressInvalid) Error() string {
+	return fmt.Sprintf("Invalid address (%s): %v", e.Addr, e.Err)
+}
+
+type ErrNetAddressLookup struct {
+	Addr string
+	Err  error
+}
+
+func (e ErrNetAddressLookup) Error() string {
+	return fmt.Sprintf("Error looking up host (%s): %v", e.Addr, e.Err)
+}

p2p/netaddress.go

@@ -19,10 +19,15 @@ import (
 // NetAddress defines information about a peer on the network
 // including its ID, IP address, and port.
 type NetAddress struct {
-	ID   ID
-	IP   net.IP
-	Port uint16
-	str  string
+	ID   ID     `json:"id"`
+	IP   net.IP `json:"ip"`
+	Port uint16 `json:"port"`
+
+	// TODO:
+	// Name string `json:"name"` // optional DNS name
+
+	// memoize .String()
+	str string
 }
 
 // IDAddressString returns id@hostPort.
@@ -56,10 +61,11 @@ func NewNetAddress(id ID, addr net.Addr) *NetAddress {
 // NewNetAddressString returns a new NetAddress using the provided address in
 // the form of "ID@IP:Port".
 // Also resolves the host if host is not an IP.
+// Errors are of type ErrNetAddressXxx where Xxx is in (NoID, Invalid, Lookup)
 func NewNetAddressString(addr string) (*NetAddress, error) {
 	spl := strings.Split(addr, "@")
 	if len(spl) < 2 {
-		return nil, fmt.Errorf("Address (%s) does not contain ID", addr)
+		return nil, ErrNetAddressNoID{addr}
 	}
 	return NewNetAddressStringWithOptionalID(addr)
 }
@@ -76,11 +82,12 @@ func NewNetAddressStringWithOptionalID(addr string) (*NetAddress, error) {
 		idStr := spl[0]
 		idBytes, err := hex.DecodeString(idStr)
 		if err != nil {
-			return nil, cmn.ErrorWrap(err, fmt.Sprintf("Address (%s) contains invalid ID", addrWithoutProtocol))
+			return nil, ErrNetAddressInvalid{addrWithoutProtocol, err}
 		}
 		if len(idBytes) != IDByteLength {
-			return nil, fmt.Errorf("Address (%s) contains ID of invalid length (%d). Should be %d hex-encoded bytes",
-				addrWithoutProtocol, len(idBytes), IDByteLength)
+			return nil, ErrNetAddressInvalid{
+				addrWithoutProtocol,
+				fmt.Errorf("invalid hex length - got %d, expected %d", len(idBytes), IDByteLength)}
 		}
 
 		id, addrWithoutProtocol = ID(idStr), spl[1]
@@ -88,7 +95,7 @@ func NewNetAddressStringWithOptionalID(addr string) (*NetAddress, error) {
 
 	host, portStr, err := net.SplitHostPort(addrWithoutProtocol)
 	if err != nil {
-		return nil, err
+		return nil, ErrNetAddressInvalid{addrWithoutProtocol, err}
 	}
 
 	ip := net.ParseIP(host)
@@ -96,7 +103,7 @@ func NewNetAddressStringWithOptionalID(addr string) (*NetAddress, error) {
 		if len(host) > 0 {
 			ips, err := net.LookupIP(host)
 			if err != nil {
-				return nil, err
+				return nil, ErrNetAddressLookup{host, err}
 			}
 			ip = ips[0]
 		}
@@ -104,7 +111,7 @@ func NewNetAddressStringWithOptionalID(addr string) (*NetAddress, error) {
 
 	port, err := strconv.ParseUint(portStr, 10, 16)
 	if err != nil {
-		return nil, err
+		return nil, ErrNetAddressInvalid{portStr, err}
 	}
 
 	na := NewNetAddressIPPort(ip, uint16(port))
@@ -120,7 +127,7 @@ func NewNetAddressStrings(addrs []string) ([]*NetAddress, []error) {
 	for _, addr := range addrs {
 		netAddr, err := NewNetAddressString(addr)
 		if err != nil {
-			errs = append(errs, fmt.Errorf("Error in address %s: %v", addr, err))
+			errs = append(errs, err)
 		} else {
 			netAddrs = append(netAddrs, netAddr)
 		}

p2p/node_info.go

@@ -21,6 +21,7 @@ func MaxNodeInfoSize() int {
 // between two peers during the Tendermint P2P handshake.
 type NodeInfo struct {
 	// Authenticate
+	// TODO: replace with NetAddress
 	ID         ID     `json:"id"`          // authenticated identifier
 	ListenAddr string `json:"listen_addr"` // accepting incoming
 
@@ -37,7 +38,9 @@ type NodeInfo struct {
 
 // Validate checks the self-reported NodeInfo is safe.
 // It returns an error if there
-// are too many Channels or any duplicate Channels.
+// are too many Channels, if there are any duplicate Channels,
+// if the ListenAddr is malformed, or if the ListenAddr is a host name
+// that can not be resolved to some IP.
 // TODO: constraints for Moniker/Other? Or is that for the UI ?
 func (info NodeInfo) Validate() error {
 	if len(info.Channels) > maxNumChannels {
@@ -52,11 +55,14 @@ func (info NodeInfo) Validate() error {
 		}
 		channels[ch] = struct{}{}
 	}
-	return nil
+
+	// ensure ListenAddr is good
+	_, err := NewNetAddressString(IDAddressString(info.ID, info.ListenAddr))
+	return err
 }
 
 // CompatibleWith checks if two NodeInfo are compatible with eachother.
-// CONTRACT: two nodes are compatible if the major/minor versions match and network match
+// CONTRACT: two nodes are compatible if the major version matches and network match
 // and they have at least one channel in common.
 func (info NodeInfo) CompatibleWith(other NodeInfo) error {
 	iMajor, iMinor, _, iErr := splitVersion(info.Version)
@@ -77,9 +83,9 @@ func (info NodeInfo) CompatibleWith(other NodeInfo) error {
 		return fmt.Errorf("Peer is on a different major version. Got %v, expected %v", oMajor, iMajor)
 	}
 
-	// minor version must match
+	// minor version can differ
 	if iMinor != oMinor {
-		return fmt.Errorf("Peer is on a different minor version. Got %v, expected %v", oMinor, iMinor)
+		// ok
 	}
 
 	// nodes must be on the same network
@@ -116,7 +122,14 @@ OUTER_LOOP:
 func (info NodeInfo) NetAddress() *NetAddress {
 	netAddr, err := NewNetAddressString(IDAddressString(info.ID, info.ListenAddr))
 	if err != nil {
-		panic(err) // everything should be well formed by now
+		switch err.(type) {
+		case ErrNetAddressLookup:
+			// XXX If the peer provided a host name  and the lookup fails here
+			// we're out of luck.
+			// TODO: use a NetAddress in NodeInfo
+		default:
+			panic(err) // everything should be well formed by now
+		}
 	}
 	return netAddr
 }

p2p/peer.go

@@ -90,7 +90,8 @@ type PeerConfig struct {
 
 	MConfig *tmconn.MConnConfig `mapstructure:"connection"`
 
-	Fuzz       bool            `mapstructure:"fuzz"` // fuzz connection (for testing)
+	DialFail   bool            `mapstructure:"dial_fail"` // for testing
+	Fuzz       bool            `mapstructure:"fuzz"`      // fuzz connection (for testing)
 	FuzzConfig *FuzzConnConfig `mapstructure:"fuzz_config"`
 }
 
@@ -101,6 +102,7 @@ func DefaultPeerConfig() *PeerConfig {
 		HandshakeTimeout: 20, // * time.Second,
 		DialTimeout:      3,  // * time.Second,
 		MConfig:          tmconn.DefaultMConnConfig(),
+		DialFail:         false,
 		Fuzz:             false,
 		FuzzConfig:       DefaultFuzzConnConfig(),
 	}
@@ -338,6 +340,10 @@ func (p *peer) String() string {
 // helper funcs
 
 func dial(addr *NetAddress, config *PeerConfig) (net.Conn, error) {
+	if config.DialFail {
+		return nil, fmt.Errorf("dial err (peerConfig.DialFail == true)")
+	}
+
 	conn, err := addr.DialTimeout(config.DialTimeout * time.Second)
 	if err != nil {
 		return nil, err

p2p/pex/addrbook.go

@@ -7,7 +7,6 @@ package pex
 import (
 	"crypto/sha256"
 	"encoding/binary"
-	"fmt"
 	"math"
 	"net"
 	"sync"
@@ -169,7 +168,9 @@ func (a *addrBook) OurAddress(addr *p2p.NetAddress) bool {
 	return ok
 }
 
-// AddAddress implements AddrBook - adds the given address as received from the given source.
+// AddAddress implements AddrBook
+// Add address to a "new" bucket. If it's already in one, only add it probabilistically.
+// Returns error if the addr is non-routable. Does not add self.
 // NOTE: addr must not be nil
 func (a *addrBook) AddAddress(addr *p2p.NetAddress, src *p2p.NetAddress) error {
 	a.mtx.Lock()
@@ -220,7 +221,11 @@ func (a *addrBook) PickAddress(biasTowardsNewAddrs int) *p2p.NetAddress {
 	a.mtx.Lock()
 	defer a.mtx.Unlock()
 
-	if a.size() == 0 {
+	bookSize := a.size()
+	if bookSize <= 0 {
+		if bookSize < 0 {
+			a.Logger.Error("Addrbook size less than 0", "nNew", a.nNew, "nOld", a.nOld)
+		}
 		return nil
 	}
 	if biasTowardsNewAddrs > 100 {
@@ -294,29 +299,35 @@ func (a *addrBook) MarkBad(addr *p2p.NetAddress) {
 
 // GetSelection implements AddrBook.
 // It randomly selects some addresses (old & new). Suitable for peer-exchange protocols.
+// Must never return a nil address.
 func (a *addrBook) GetSelection() []*p2p.NetAddress {
 	a.mtx.Lock()
 	defer a.mtx.Unlock()
 
-	if a.size() == 0 {
+	bookSize := a.size()
+	if bookSize <= 0 {
+		if bookSize < 0 {
+			a.Logger.Error("Addrbook size less than 0", "nNew", a.nNew, "nOld", a.nOld)
+		}
 		return nil
 	}
 
-	allAddr := make([]*p2p.NetAddress, a.size())
+	numAddresses := cmn.MaxInt(
+		cmn.MinInt(minGetSelection, bookSize),
+		bookSize*getSelectionPercent/100)
+	numAddresses = cmn.MinInt(maxGetSelection, numAddresses)
+
+	// XXX: instead of making a list of all addresses, shuffling, and slicing a random chunk,
+	// could we just select a random numAddresses of indexes?
+	allAddr := make([]*p2p.NetAddress, bookSize)
 	i := 0
 	for _, ka := range a.addrLookup {
 		allAddr[i] = ka.Addr
 		i++
 	}
 
-	numAddresses := cmn.MaxInt(
-		cmn.MinInt(minGetSelection, len(allAddr)),
-		len(allAddr)*getSelectionPercent/100)
-	numAddresses = cmn.MinInt(maxGetSelection, numAddresses)
-
 	// 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 := cmn.RandIntn(len(allAddr)-i) + i
@@ -329,6 +340,7 @@ func (a *addrBook) GetSelection() []*p2p.NetAddress {
 
 // GetSelectionWithBias implements AddrBook.
 // It randomly selects some addresses (old & new). Suitable for peer-exchange protocols.
+// Must never return a nil address.
 //
 // Each address is picked randomly from an old or new bucket according to the
 // biasTowardsNewAddrs argument, which must be between [0, 100] (or else is truncated to
@@ -338,7 +350,11 @@ func (a *addrBook) GetSelectionWithBias(biasTowardsNewAddrs int) []*p2p.NetAddre
 	a.mtx.Lock()
 	defer a.mtx.Unlock()
 
-	if a.size() == 0 {
+	bookSize := a.size()
+	if bookSize <= 0 {
+		if bookSize < 0 {
+			a.Logger.Error("Addrbook size less than 0", "nNew", a.nNew, "nOld", a.nOld)
+		}
 		return nil
 	}
 
@@ -350,8 +366,8 @@ func (a *addrBook) GetSelectionWithBias(biasTowardsNewAddrs int) []*p2p.NetAddre
 	}
 
 	numAddresses := cmn.MaxInt(
-		cmn.MinInt(minGetSelection, a.size()),
-		a.size()*getSelectionPercent/100)
+		cmn.MinInt(minGetSelection, bookSize),
+		bookSize*getSelectionPercent/100)
 	numAddresses = cmn.MinInt(maxGetSelection, numAddresses)
 
 	selection := make([]*p2p.NetAddress, numAddresses)
@@ -487,11 +503,11 @@ func (a *addrBook) getBucket(bucketType byte, bucketIdx int) map[string]*knownAd
 
 // Adds ka to new bucket. Returns false if it couldn't do it cuz buckets full.
 // NOTE: currently it always returns true.
-func (a *addrBook) addToNewBucket(ka *knownAddress, bucketIdx int) bool {
+func (a *addrBook) addToNewBucket(ka *knownAddress, bucketIdx int) {
 	// Sanity check
 	if ka.isOld() {
-		a.Logger.Error(cmn.Fmt("Cannot add address already in old bucket to a new bucket: %v", ka))
-		return false
+		a.Logger.Error("Failed Sanity Check! Cant add old address to new bucket", "ka", ka, "bucket", bucketIdx)
+		return
 	}
 
 	addrStr := ka.Addr.String()
@@ -499,7 +515,7 @@ func (a *addrBook) addToNewBucket(ka *knownAddress, bucketIdx int) bool {
 
 	// Already exists?
 	if _, ok := bucket[addrStr]; ok {
-		return true
+		return
 	}
 
 	// Enforce max addresses.
@@ -517,8 +533,6 @@ func (a *addrBook) addToNewBucket(ka *knownAddress, bucketIdx int) bool {
 
 	// Add it to addrLookup
 	a.addrLookup[ka.ID()] = ka
-
-	return true
 }
 
 // Adds ka to old bucket. Returns false if it couldn't do it cuz buckets full.
@@ -605,19 +619,22 @@ func (a *addrBook) pickOldest(bucketType byte, bucketIdx int) *knownAddress {
 // adds the address to a "new" bucket. if its already in one,
 // it only adds it probabilistically
 func (a *addrBook) addAddress(addr, src *p2p.NetAddress) error {
+	if addr == nil || src == nil {
+		return ErrAddrBookNilAddr{addr, src}
+	}
+
 	if a.routabilityStrict && !addr.Routable() {
-		return fmt.Errorf("Cannot add non-routable address %v", addr)
+		return ErrAddrBookNonRoutable{addr}
 	}
+	// TODO: we should track ourAddrs by ID and by IP:PORT and refuse both.
 	if _, ok := a.ourAddrs[addr.String()]; ok {
-		// Ignore our own listener address.
-		return fmt.Errorf("Cannot add ourselves with address %v", addr)
+		return ErrAddrBookSelf{addr}
 	}
 
 	ka := a.addrLookup[addr.ID]
-
 	if ka != nil {
-		// Already old.
-		if ka.isOld() {
+		// If its already old and the addr is the same, ignore it.
+		if ka.isOld() && ka.Addr.Equals(addr) {
 			return nil
 		}
 		// Already in max new buckets.
@@ -634,12 +651,7 @@ func (a *addrBook) addAddress(addr, src *p2p.NetAddress) error {
 	}
 
 	bucket := a.calcNewBucket(addr, src)
-	added := a.addToNewBucket(ka, bucket)
-	if !added {
-		a.Logger.Info("Can't add new address, addr book is full", "address", addr, "total", a.size())
-	}
-
-	a.Logger.Info("Added new address", "address", addr, "total", a.size())
+	a.addToNewBucket(ka, bucket)
 	return nil
 }
 
@@ -674,8 +686,6 @@ func (a *addrBook) moveToOld(ka *knownAddress) {
 		return
 	}
 
-	// Remember one of the buckets in which ka is in.
-	freedBucket := ka.Buckets[0]
 	// Remove from all (new) buckets.
 	a.removeFromAllBuckets(ka)
 	// It's officially old now.
@@ -685,20 +695,13 @@ func (a *addrBook) moveToOld(ka *knownAddress) {
 	oldBucketIdx := a.calcOldBucket(ka.Addr)
 	added := a.addToOldBucket(ka, oldBucketIdx)
 	if !added {
-		// No room, must evict something
+		// No room; move the oldest to a new bucket
 		oldest := a.pickOldest(bucketTypeOld, oldBucketIdx)
 		a.removeFromBucket(oldest, bucketTypeOld, oldBucketIdx)
-		// Find new bucket to put oldest in
 		newBucketIdx := a.calcNewBucket(oldest.Addr, oldest.Src)
-		added := a.addToNewBucket(oldest, newBucketIdx)
-		// No space in newBucket either, just put it in freedBucket from above.
-		if !added {
-			added := a.addToNewBucket(oldest, freedBucket)
-			if !added {
-				a.Logger.Error(cmn.Fmt("Could not migrate oldest %v to freedBucket %v", oldest, freedBucket))
-			}
-		}
-		// Finally, add to bucket again.
+		a.addToNewBucket(oldest, newBucketIdx)
+
+		// Finally, add our ka to old bucket again.
 		added = a.addToOldBucket(ka, oldBucketIdx)
 		if !added {
 			a.Logger.Error(cmn.Fmt("Could not re-add ka %v to oldBucketIdx %v", ka, oldBucketIdx))

p2p/pex/errors.go

@@ -0,0 +1,32 @@
+package pex
+
+import (
+	"fmt"
+
+	"github.com/tendermint/tendermint/p2p"
+)
+
+type ErrAddrBookNonRoutable struct {
+	Addr *p2p.NetAddress
+}
+
+func (err ErrAddrBookNonRoutable) Error() string {
+	return fmt.Sprintf("Cannot add non-routable address %v", err.Addr)
+}
+
+type ErrAddrBookSelf struct {
+	Addr *p2p.NetAddress
+}
+
+func (err ErrAddrBookSelf) Error() string {
+	return fmt.Sprintf("Cannot add ourselves with address %v", err.Addr)
+}
+
+type ErrAddrBookNilAddr struct {
+	Addr *p2p.NetAddress
+	Src  *p2p.NetAddress
+}
+
+func (err ErrAddrBookNilAddr) Error() string {
+	return fmt.Sprintf("Cannot add a nil address. Got (addr, src) = (%v, %v)", err.Addr, err.Src)
+}

p2p/pex/known_address.go

@@ -106,7 +106,6 @@ 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
@@ -115,14 +114,15 @@ func (ka *knownAddress) isBad() bool {
 	}
 
 	// Has been attempted in the last minute --> good
-	if ka.LastAttempt.Before(time.Now().Add(-1 * time.Minute)) {
+	if ka.LastAttempt.After(time.Now().Add(-1 * time.Minute)) {
 		return false
 	}
 
+	// TODO: From the future?
+
 	// 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)) {
+	// TODO: should be a timestamp of last seen, not just last attempt
+	if ka.LastAttempt.Before(time.Now().Add(-1 * numMissingDays * time.Hour * 24)) {
 		return true
 	}
 
@@ -132,7 +132,6 @@ 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

p2p/pex/params.go

@@ -50,6 +50,6 @@ const (
 	minGetSelection = 32
 
 	// max addresses returned by GetSelection
-	// NOTE: this must match "maxPexMessageSize"
+	// NOTE: this must match "maxMsgSize"
 	maxGetSelection = 250
 )

p2p/pex/pex_reactor.go

@@ -20,11 +20,18 @@ const (
 	// PexChannel is a channel for PEX messages
 	PexChannel = byte(0x00)
 
-	maxMsgSize = 1048576 // 1MB
+	// over-estimate of max NetAddress size
+	// hexID (40) + IP (16) + Port (2) + Name (100) ...
+	// NOTE: dont use massive DNS name ..
+	maxAddressSize = 256
+
+	// NOTE: amplificaiton factor!
+	// small request results in up to maxMsgSize response
+	maxMsgSize = maxAddressSize * maxGetSelection
 
 	// ensure we have enough peers
 	defaultEnsurePeersPeriod   = 30 * time.Second
-	defaultMinNumOutboundPeers = 10
+	defaultMinNumOutboundPeers = p2p.DefaultMinNumOutboundPeers
 
 	// Seed/Crawler constants
 
@@ -61,7 +68,7 @@ type PEXReactor struct {
 
 	book              AddrBook
 	config            *PEXReactorConfig
-	ensurePeersPeriod time.Duration
+	ensurePeersPeriod time.Duration // TODO: should go in the config
 
 	// maps to prevent abuse
 	requestsSent         *cmn.CMap // ID->struct{}: unanswered send requests
@@ -70,6 +77,12 @@ type PEXReactor struct {
 	attemptsToDial sync.Map // address (string) -> {number of attempts (int), last time dialed (time.Time)}
 }
 
+func (pexR *PEXReactor) minReceiveRequestInterval() time.Duration {
+	// NOTE: must be less than ensurePeersPeriod, otherwise we'll request
+	// peers too quickly from others and they'll think we're bad!
+	return pexR.ensurePeersPeriod / 3
+}
+
 // PEXReactorConfig holds reactor specific configuration data.
 type PEXReactorConfig struct {
 	// Seed/Crawler mode
@@ -113,6 +126,7 @@ func (r *PEXReactor) OnStart() error {
 	}
 
 	// return err if user provided a bad seed address
+	// or a host name that we cant resolve
 	if err := r.checkSeeds(); err != nil {
 		return err
 	}
@@ -155,16 +169,30 @@ func (r *PEXReactor) AddPeer(p Peer) {
 			r.RequestAddrs(p)
 		}
 	} else {
-		// For inbound peers, the peer is its own source,
-		// and its NodeInfo has already been validated.
-		// Let the ensurePeersRoutine handle asking for more
-		// peers when we need - we don't trust inbound peers as much.
+		// inbound peer is its own source
 		addr := p.NodeInfo().NetAddress()
-		if !isAddrPrivate(addr, r.config.PrivatePeerIDs) {
-			err := r.book.AddAddress(addr, addr)
-			if err != nil {
-				r.Logger.Error("Failed to add new address", "err", err)
-			}
+		src := addr
+
+		// ignore private addrs
+		if isAddrPrivate(addr, r.config.PrivatePeerIDs) {
+			return
+		}
+
+		// add to book. dont RequestAddrs right away because
+		// we don't trust inbound as much - let ensurePeersRoutine handle it.
+		err := r.book.AddAddress(addr, src)
+		r.logErrAddrBook(err)
+	}
+}
+
+func (r *PEXReactor) logErrAddrBook(err error) {
+	if err != nil {
+		switch err.(type) {
+		case ErrAddrBookNilAddr:
+			r.Logger.Error("Failed to add new address", "err", err)
+		default:
+			// non-routable, self, full book, etc.
+			r.Logger.Debug("Failed to add new address", "err", err)
 		}
 	}
 }
@@ -195,6 +223,10 @@ func (r *PEXReactor) Receive(chID byte, src Peer, msgBytes []byte) {
 		}
 
 		// Seeds disconnect after sending a batch of addrs
+		// NOTE: this is a prime candidate for amplification attacks
+		// so it's important we
+		// 1) restrict how frequently peers can request
+		// 2) limit the output size
 		if r.config.SeedMode {
 			r.SendAddrs(src, r.book.GetSelectionWithBias(biasToSelectNewPeers))
 			r.Switch.StopPeerGracefully(src)
@@ -213,6 +245,7 @@ func (r *PEXReactor) Receive(chID byte, src Peer, msgBytes []byte) {
 	}
 }
 
+// enforces a minimum amount of time between requests
 func (r *PEXReactor) receiveRequest(src Peer) error {
 	id := string(src.ID())
 	v := r.lastReceivedRequests.Get(id)
@@ -232,8 +265,14 @@ func (r *PEXReactor) receiveRequest(src Peer) error {
 	}
 
 	now := time.Now()
-	if now.Sub(lastReceived) < r.ensurePeersPeriod/3 {
-		return fmt.Errorf("Peer (%v) is sending too many PEX requests. Disconnecting", src.ID())
+	minInterval := r.minReceiveRequestInterval()
+	if now.Sub(lastReceived) < minInterval {
+		return fmt.Errorf("Peer (%v) send next PEX request too soon. lastReceived: %v, now: %v, minInterval: %v. Disconnecting",
+			src.ID(),
+			lastReceived,
+			now,
+			minInterval,
+		)
 	}
 	r.lastReceivedRequests.Set(id, now)
 	return nil
@@ -254,22 +293,30 @@ func (r *PEXReactor) RequestAddrs(p Peer) {
 // request for this peer and deletes the open request.
 // If there's no open request for the src peer, it returns an error.
 func (r *PEXReactor) ReceiveAddrs(addrs []*p2p.NetAddress, src Peer) error {
-	id := string(src.ID())
 
+	id := string(src.ID())
 	if !r.requestsSent.Has(id) {
 		return cmn.NewError("Received unsolicited pexAddrsMessage")
 	}
-
 	r.requestsSent.Delete(id)
 
 	srcAddr := src.NodeInfo().NetAddress()
 	for _, netAddr := range addrs {
-		if netAddr != nil && !isAddrPrivate(netAddr, r.config.PrivatePeerIDs) {
-			err := r.book.AddAddress(netAddr, srcAddr)
-			if err != nil {
-				r.Logger.Error("Failed to add new address", "err", err)
-			}
+		// NOTE: GetSelection methods should never return nil addrs
+		if netAddr == nil {
+			return cmn.NewError("received nil addr")
+		}
+
+		// ignore private peers
+		// TODO: give private peers to AddrBook so it can enforce this on AddAddress.
+		// We'd then have to check for ErrPrivatePeer on AddAddress here, which is
+		// an error we just ignore (maybe peer is probing us for our private peers :P)
+		if isAddrPrivate(netAddr, r.config.PrivatePeerIDs) {
+			continue
 		}
+
+		err := r.book.AddAddress(netAddr, srcAddr)
+		r.logErrAddrBook(err)
 	}
 	return nil
 }
@@ -360,6 +407,9 @@ func (r *PEXReactor) ensurePeers() {
 		if connected := r.Switch.Peers().Has(try.ID); connected {
 			continue
 		}
+		// TODO: consider moving some checks from toDial into here
+		// so we don't even consider dialing peers that we want to wait
+		// before dialling again, or have dialed too many times already
 		r.Logger.Info("Will dial address", "addr", try)
 		toDial[try.ID] = try
 	}
@@ -387,13 +437,17 @@ func (r *PEXReactor) ensurePeers() {
 	}
 }
 
-func (r *PEXReactor) dialPeer(addr *p2p.NetAddress) {
-	var attempts int
-	var lastDialed time.Time
-	if lAttempts, attempted := r.attemptsToDial.Load(addr.DialString()); attempted {
-		attempts = lAttempts.(_attemptsToDial).number
-		lastDialed = lAttempts.(_attemptsToDial).lastDialed
+func (r *PEXReactor) dialAttemptsInfo(addr *p2p.NetAddress) (attempts int, lastDialed time.Time) {
+	_attempts, ok := r.attemptsToDial.Load(addr.DialString())
+	if !ok {
+		return
 	}
+	atd := _attempts.(_attemptsToDial)
+	return atd.number, atd.lastDialed
+}
+
+func (r *PEXReactor) dialPeer(addr *p2p.NetAddress) {
+	attempts, lastDialed := r.dialAttemptsInfo(addr)
 
 	if attempts > maxAttemptsToDial {
 		r.Logger.Error("Reached max attempts to dial", "addr", addr, "attempts", attempts)
@@ -590,7 +644,7 @@ func (r *PEXReactor) attemptDisconnects() {
 	}
 }
 
-// isAddrPrivate returns true if addr is private.
+// isAddrPrivate returns true if addr.ID is a private ID.
 func isAddrPrivate(addr *p2p.NetAddress, privatePeerIDs []string) bool {
 	for _, id := range privatePeerIDs {
 		if string(addr.ID) == id {

p2p/switch.go

@@ -26,6 +26,10 @@ const (
 	// ie. 3**10 = 16hrs
 	reconnectBackOffAttempts    = 10
 	reconnectBackOffBaseSeconds = 3
+
+	// keep at least this many outbound peers
+	// TODO: move to config
+	DefaultMinNumOutboundPeers = 10
 )
 
 //-----------------------------------------------------------------------------
@@ -59,6 +63,7 @@ type Switch struct {
 	reactorsByCh map[byte]Reactor
 	peers        *PeerSet
 	dialing      *cmn.CMap
+	reconnecting *cmn.CMap
 	nodeInfo     NodeInfo // our node info
 	nodeKey      *NodeKey // our node privkey
 	addrBook     AddrBook
@@ -79,6 +84,7 @@ func NewSwitch(config *cfg.P2PConfig) *Switch {
 		reactorsByCh: make(map[byte]Reactor),
 		peers:        NewPeerSet(),
 		dialing:      cmn.NewCMap(),
+		reconnecting: cmn.NewCMap(),
 	}
 
 	// Ensure we have a completely undeterministic PRNG.
@@ -258,7 +264,8 @@ func (sw *Switch) StopPeerForError(peer Peer, reason interface{}) {
 	sw.stopAndRemovePeer(peer, reason)
 
 	if peer.IsPersistent() {
-		go sw.reconnectToPeer(peer)
+		// NOTE: this is the self-reported addr, not the original we dialed
+		go sw.reconnectToPeer(peer.NodeInfo().NetAddress())
 	}
 }
 
@@ -277,34 +284,41 @@ func (sw *Switch) stopAndRemovePeer(peer Peer, reason interface{}) {
 	}
 }
 
-// reconnectToPeer tries to reconnect to the peer, first repeatedly
+// reconnectToPeer tries to reconnect to the addr, first repeatedly
 // with a fixed interval, then with exponential backoff.
 // If no success after all that, it stops trying, and leaves it
-// to the PEX/Addrbook to find the peer again
-func (sw *Switch) reconnectToPeer(peer Peer) {
-	// NOTE this will connect to the self reported address,
-	// not necessarily the original we dialed
-	netAddr := peer.NodeInfo().NetAddress()
+// to the PEX/Addrbook to find the peer with the addr again
+// NOTE: this will keep trying even if the handshake or auth fails.
+// TODO: be more explicit with error types so we only retry on certain failures
+//  - ie. if we're getting ErrDuplicatePeer we can stop
+//  	because the addrbook got us the peer back already
+func (sw *Switch) reconnectToPeer(addr *NetAddress) {
+	if sw.reconnecting.Has(string(addr.ID)) {
+		return
+	}
+	sw.reconnecting.Set(string(addr.ID), addr)
+	defer sw.reconnecting.Delete(string(addr.ID))
+
 	start := time.Now()
-	sw.Logger.Info("Reconnecting to peer", "peer", peer)
+	sw.Logger.Info("Reconnecting to peer", "addr", addr)
 	for i := 0; i < reconnectAttempts; i++ {
 		if !sw.IsRunning() {
 			return
 		}
 
-		err := sw.DialPeerWithAddress(netAddr, true)
-		if err != nil {
-			sw.Logger.Info("Error reconnecting to peer. Trying again", "tries", i, "err", err, "peer", peer)
-			// sleep a set amount
-			sw.randomSleep(reconnectInterval)
-			continue
-		} else {
-			return
+		err := sw.DialPeerWithAddress(addr, true)
+		if err == nil {
+			return // success
 		}
+
+		sw.Logger.Info("Error reconnecting to peer. Trying again", "tries", i, "err", err, "addr", addr)
+		// sleep a set amount
+		sw.randomSleep(reconnectInterval)
+		continue
 	}
 
 	sw.Logger.Error("Failed to reconnect to peer. Beginning exponential backoff",
-		"peer", peer, "elapsed", time.Since(start))
+		"addr", addr, "elapsed", time.Since(start))
 	for i := 0; i < reconnectBackOffAttempts; i++ {
 		if !sw.IsRunning() {
 			return
@@ -313,13 +327,13 @@ func (sw *Switch) reconnectToPeer(peer Peer) {
 		// sleep an exponentially increasing amount
 		sleepIntervalSeconds := math.Pow(reconnectBackOffBaseSeconds, float64(i))
 		sw.randomSleep(time.Duration(sleepIntervalSeconds) * time.Second)
-		err := sw.DialPeerWithAddress(netAddr, true)
+		err := sw.DialPeerWithAddress(addr, true)
 		if err == nil {
 			return // success
 		}
-		sw.Logger.Info("Error reconnecting to peer. Trying again", "tries", i, "err", err, "peer", peer)
+		sw.Logger.Info("Error reconnecting to peer. Trying again", "tries", i, "err", err, "addr", addr)
 	}
-	sw.Logger.Error("Failed to reconnect to peer. Giving up", "peer", peer, "elapsed", time.Since(start))
+	sw.Logger.Error("Failed to reconnect to peer. Giving up", "addr", addr, "elapsed", time.Since(start))
 }
 
 // SetAddrBook allows to set address book on Switch.
@@ -344,6 +358,8 @@ func (sw *Switch) IsDialing(id ID) bool {
 }
 
 // DialPeersAsync dials a list of peers asynchronously in random order (optionally, making them persistent).
+// Used to dial peers from config on startup or from unsafe-RPC (trusted sources).
+// TODO: remove addrBook arg since it's now set on the switch
 func (sw *Switch) DialPeersAsync(addrBook AddrBook, peers []string, persistent bool) error {
 	netAddrs, errs := NewNetAddressStrings(peers)
 	// only log errors, dial correct addresses
@@ -353,7 +369,10 @@ func (sw *Switch) DialPeersAsync(addrBook AddrBook, peers []string, persistent b
 
 	ourAddr := sw.nodeInfo.NetAddress()
 
-	// TODO: move this out of here ?
+	// TODO: this code feels like it's in the wrong place.
+	// The integration tests depend on the addrBook being saved
+	// right away but maybe we can change that. Recall that
+	// the addrBook is only written to disk every 2min
 	if addrBook != nil {
 		// add peers to `addrBook`
 		for _, netAddr := range netAddrs {
@@ -375,15 +394,21 @@ func (sw *Switch) DialPeersAsync(addrBook AddrBook, peers []string, persistent b
 		go func(i int) {
 			j := perm[i]
 
+			addr := netAddrs[j]
 			// do not dial ourselves
-			if netAddrs[j].Same(ourAddr) {
+			if addr.Same(ourAddr) {
 				return
 			}
 
 			sw.randomSleep(0)
-			err := sw.DialPeerWithAddress(netAddrs[j], persistent)
+			err := sw.DialPeerWithAddress(addr, persistent)
 			if err != nil {
-				sw.Logger.Error("Error dialing peer", "err", err)
+				switch err {
+				case ErrSwitchConnectToSelf, ErrSwitchDuplicatePeer:
+					sw.Logger.Debug("Error dialing peer", "err", err)
+				default:
+					sw.Logger.Error("Error dialing peer", "err", err)
+				}
 			}
 		}(i)
 	}
@@ -444,7 +469,8 @@ func (sw *Switch) listenerRoutine(l Listener) {
 		}
 
 		// ignore connection if we already have enough
-		maxPeers := sw.config.MaxNumPeers
+		// leave room for MinNumOutboundPeers
+		maxPeers := sw.config.MaxNumPeers - DefaultMinNumOutboundPeers
 		if maxPeers <= sw.peers.Size() {
 			sw.Logger.Info("Ignoring inbound connection: already have enough peers", "address", inConn.RemoteAddr().String(), "numPeers", sw.peers.Size(), "max", maxPeers)
 			continue
@@ -477,16 +503,19 @@ func (sw *Switch) addInboundPeerWithConfig(conn net.Conn, config *PeerConfig) er
 
 // dial the peer; make secret connection; authenticate against the dialed ID;
 // add the peer.
+// if dialing fails, start the reconnect loop. If handhsake fails, its over.
+// If peer is started succesffuly, reconnectLoop will start when StopPeerForError is called
 func (sw *Switch) addOutboundPeerWithConfig(addr *NetAddress, config *PeerConfig, persistent bool) error {
 	sw.Logger.Info("Dialing peer", "address", addr)
 	peerConn, err := newOutboundPeerConn(addr, config, persistent, sw.nodeKey.PrivKey)
 	if err != nil {
-		sw.Logger.Error("Failed to dial peer", "address", addr, "err", err)
+		if persistent {
+			go sw.reconnectToPeer(addr)
+		}
 		return err
 	}
 
 	if err := sw.addPeer(peerConn); err != nil {
-		sw.Logger.Error("Failed to add peer", "address", addr, "err", err)
 		peerConn.CloseConn()
 		return err
 	}

p2p/switch_test.go

@@ -315,6 +315,29 @@ func TestSwitchReconnectsToPersistentPeer(t *testing.T) {
 	}
 	assert.NotZero(npeers)
 	assert.False(peer.IsRunning())
+
+	// simulate another remote peer
+	rp = &remotePeer{PrivKey: crypto.GenPrivKeyEd25519(), Config: DefaultPeerConfig()}
+	rp.Start()
+	defer rp.Stop()
+
+	// simulate first time dial failure
+	peerConfig := DefaultPeerConfig()
+	peerConfig.DialFail = true
+	err = sw.addOutboundPeerWithConfig(rp.Addr(), peerConfig, true)
+	require.NotNil(err)
+
+	// DialPeerWithAddres - sw.peerConfig resets the dialer
+
+	// TODO: same as above
+	for i := 0; i < 20; i++ {
+		time.Sleep(250 * time.Millisecond)
+		npeers = sw.Peers().Size()
+		if npeers > 1 {
+			break
+		}
+	}
+	assert.EqualValues(2, npeers)
 }
 
 func TestSwitchFullConnectivity(t *testing.T) {

p2p/test_util.go

@@ -49,6 +49,8 @@ func CreateRoutableAddr() (addr string, netAddr *NetAddress) {
 //------------------------------------------------------------------
 // Connects switches via arbitrary net.Conn. Used for testing.
 
+const TEST_HOST = "localhost"
+
 // MakeConnectedSwitches returns n switches, connected according to the connect func.
 // If connect==Connect2Switches, the switches will be fully connected.
 // initSwitch defines how the i'th switch should be initialized (ie. with what reactors).
@@ -56,7 +58,7 @@ func CreateRoutableAddr() (addr string, netAddr *NetAddress) {
 func MakeConnectedSwitches(cfg *cfg.P2PConfig, n int, initSwitch func(int, *Switch) *Switch, connect func([]*Switch, int, int)) []*Switch {
 	switches := make([]*Switch, n)
 	for i := 0; i < n; i++ {
-		switches[i] = MakeSwitch(cfg, i, "testing", "123.123.123", initSwitch)
+		switches[i] = MakeSwitch(cfg, i, TEST_HOST, "123.123.123", initSwitch)
 	}
 
 	if err := StartSwitches(switches); err != nil {

rpc/core/net.go

@@ -23,6 +23,7 @@ import (
 // {
 // 	"error": "",
 // 	"result": {
+//		"n_peers": 0,
 // 		"peers": [],
 // 		"listeners": [
 // 			"Listener(@10.0.2.15:46656)"
@@ -47,9 +48,13 @@ func NetInfo() (*ctypes.ResultNetInfo, error) {
 			ConnectionStatus: peer.Status(),
 		})
 	}
+	// TODO: Should we include PersistentPeers and Seeds in here?
+	// PRO: useful info
+	// CON: privacy
 	return &ctypes.ResultNetInfo{
 		Listening: listening,
 		Listeners: listeners,
+		NPeers:    len(peers),
 		Peers:     peers,
 	}, nil
 }

rpc/core/types/responses.go

@@ -100,6 +100,7 @@ func (s *ResultStatus) TxIndexEnabled() bool {
 type ResultNetInfo struct {
 	Listening bool     `json:"listening"`
 	Listeners []string `json:"listeners"`
+	NPeers    int      `json:"n_peers"`
 	Peers     []Peer   `json:"peers"`
 }
 

version/version.go

@@ -4,13 +4,13 @@ package version
 const (
 	Maj = "0"
 	Min = "19"
-	Fix = "1"
+	Fix = "2"
 )
 
 var (
 	// Version is the current version of Tendermint
 	// Must be a string because scripts like dist.sh read this file.
-	Version = "0.19.1"
+	Version = "0.19.2"
 
 	// GitCommit is the current HEAD set using ldflags.
 	GitCommit string