Add Discovery

[aschmahmann]

This adds bootstrapping and the ability for rediscovery into the PubSub system as described in libp2p/go-libp2p-pubsub-router#28.

Once we go forward with this we can modify go-libp2p-pubsub-router to utilize an arbitrary discovery mechanism instead of the built in content routing based discovery. We likely will also need to implement a number of encapsulating discovery wrappers (e.g. tiered, parallel, etc.) like we have for content routing.

@raulk @vyzo @Stebalien

[vyzo]

Let's move the discovery logic to its own component/file.

[vyzo]

Just to clarify the change request, I would like the discovery logic in its own component instead of piling up on the pubsub struct (and file).
See how we handle validation.

[aschmahmann]

Just to clarify the change request, I would like the discovery logic in its own component instead of piling up on the pubsub struct (and file).
See how we handle validation.

Seems reasonable, however, it like makes fixing #184 (comment) even more difficult (i.e. without looking like a mess of private channels between the PubSub and discover structs). You ok if we punt on that for now and just re-add the waitgroup?

[vyzo]

It's great progress to see the discovery stuffs in its own module, but I am totally confused about this new peer notification stuff you are adding.
That looks completely redundant, as we already have logic to handle this with connection notifications.

[aschmahmann]

It's great progress to see the discovery stuffs in its own module, but I am totally confused about this new peer notification stuff you are adding.
That looks completely redundant, as we already have logic to handle this with connection notifications.

This is needed in order to solve #184 (comment). In particular, if we initialize pubsub and the first thing we do is Publish to a topic then we'll start bootstrapping that topic before we actually send the message. If we send the message too soon it'll just be dropped since there won't be anyone in the network (or a large enough portion of the network) to hear the message. We used to just wait a little bit (250ms) between the end of network.Connect and when PubSub has received a Subscribe message and hope that was enough. This peer notification mechanism let's us actually know when we've finished running our bootstrap function.

[vyzo]

The pubsub mainloop should have no details of discovery leaking.
All it needs is a single event, please track the peers internally in discovery if you have to.

[vyzo]

this is almost ready to merge I would say, let's get another pass from @Stebalien as well.

[Stebalien]

We're going to spawn a bunch of goroutines. We really do need to listen on peer join/leave events to determine when we're done bootstrapping. At the moment:

1. This is going to fire once the DHT query completes, regardless of whether or not we're done connecting to peers.
2. We're going to hit that 100ms sleep.
3. We're then going to restart the loop, see that we aren't "ready" because we haven't finished connecting yet.
4. We're going to spawn another discovery process.
5. We're going to spawn a bunch of goroutines to connect to the same peers.

---

The right way to do this is:

1. When we want more peers, signal that we want to discover more peers.
2. When we don't need any more peers, signal that we're happy with the number of peers we have.

---

Note: We can probably also find a way to hack in a fix but, at the very least, we can't spawn a bunch of goroutines connecting to the same peers.

[aschmahmann]

Emitting an "I don't need more peers" signal is likely much more invasive then just checking from the outside. However, if we're certain that the "how many peers" functions that we care about is static per PubSub instance then we can use the PeerJoin events instead of time.Sleep(100 ms) (although if the number of peers we wait on can change over time this won't work).

We're going to spawn another discovery process.

This won't be very expensive since it should be backed by a backoff cache of some sort anyhow.

We're going to spawn a bunch of goroutines to connect to the same peers.

I'm fine replacing this with storing a local cache of queried peers. I only left it this way bc I though when we talked about this previously that you didn't want to store that state locally.

[Stebalien]

We discussed this on a call. It's going to be fine as-is as long as we remember which peers we're currently dialing and/or have recently dialed.

[Stebalien]

🚀

[vyzo]

This looks good; just a minor bug and and open question about the MinTopicSize returend function being called potentially outside the event loop.

[Stebalien]

The history is nasty enough we should probably rebase and squash (or at least rebase to get rid of the merges).

[Stebalien]

🚀
👏👏👏