blockchain: BlockchainReactor.AddPeer doesn't add peer to its pool

[odeke-em]

I just hopped on to work on finishing PR #648 and while reading through the code I noticed that the code for AddPeer

tendermint/blockchain/reactor.go

Lines 114 to 118 in 7f8240d

	func (bcR *BlockchainReactor) AddPeer(peer p2p.Peer) {
	if !peer.Send(BlockchainChannel, struct{ BlockchainMessage }{&bcStatusResponseMessage{bcR.store.Height()}}) {
	// doing nothing, will try later in `poolRoutine`
	}
	}

Just tries to send the peer our state, doesn't add the peer to the pool.

However, the code in RemovePeer pops the peer from the pool

tendermint/blockchain/reactor.go

Lines 121 to 123 in 7f8240d

	func (bcR *BlockchainReactor) RemovePeer(peer p2p.Peer, reason interface{}) {
	bcR.pool.RemovePeer(peer.Key())
	}

Am I mistaken that we aren't properly adding a peer to the pool or is there some reasoning for this? Also I don't see any tests for AddPeer nor for RemovePeer.

[melekes]

We add peer to the pool upon receiving bcStatusResponseMessage https://github.com/tendermint/tendermint/blob/develop/blockchain/reactor.go#L159

but yeah, it's a bit confusing

[ebuchman]

We should update the comments to more clearly reflect what's going on - the peer doesnt get added to the pool until we've received a bc msg.

So this isn't really an issue as is, but we should clarify the comments and add tests