gossip methods: renames and predicate adjustment

[raulk]

This PR proposes minor renames for accuracy.

It also uses the predicate on ps.getPeers to exclude the exclusion set, to sure we source as many as D peers to push gossip to. With the current logic, we might select peers that later end up being excluded, resulting in us gossiping to less than D peers.

[aschmahmann]

@raulk No problem with these changes, however when I've spoken to @vyzo about changing the predicate in the past he has mentioned that this is an intentional change. So we may need to first decide whether the real issue is docs or implementation (i.e. libp2p/docs#66).

My understanding of the argument to leave the predicate alone is that we'd actually like the effective D peers we message to decrease as the percentage of peers we know about are ones in our mesh increases. We do this because the whole point of gossip is to send out messages to peers who likely have not seen our message (note: if a peer sends us a GRAFT between message send and the first gossip they are guaranteed to not to receive the message from us), and so if we have 18 peers with 12 in our mesh we don't really feel the need to send the messageID to all 6 of the remaining peers.

If we are onboard with the above logic (or if @vyzo has other points that I'm missing) we must fix the documentation (@vyzo disagrees and says this is just an implementation optimization), otherwise people implementing simulations will achieve different results from us. Additionally, if we wanted we could make the logic a little more predictable by using the proposed predicate (filtering based on existing peers), but setting the number to be based on the fraction of peers not in the mesh e.g. D*(len(topicPeers) - len(meshPeers))/len(topicPeers)

No complaint about simply adopting this PR if we do not think the above argument is sufficient. However, we should also make sure to change it in the simulations so that they stay consistent.

EDIT: Also, if we do this change we should make sure to propagate this information to the JS and Rust implementations.

[raulk]

@aschmahmann Thoughts:

• Yeah, if we're seeking a balancing effect here it needs to be formally captured. Right now it's an unspoken truth.
• This is asymmetrically biased. If we're above D, we penalise/suppress some gossip, but if we're below D, we do not boost gossip.
• To me, this seems like the inverse heuristic of what should be happening, and it can have a detrimental emergent effect.
  • If a peer is meshing above D, it's probably because it's a well-advertised and discoverable peer for that topic.
  • As a result, it is likely a hot node in the dissemination graph for that topic.
  • For the benefit of the network, we'd want that peer to emit more gossip to nodes that didn't make it into its mesh, not less.

---

I think Rust already behaves the way proposed herein: https://github.com/libp2p/rust-libp2p/pull/898/files#diff-88fd6ff4aa853343221bf015d3c5a32bR768 (follow the breadcrumbs).

gossipsub-js seems to not impose an upper limit on gossip: https://github.com/ChainSafe/gossipsub-js/blob/a6bd1850085176b912bc9e1c350122e34c8eefed/src/index.js#L547-L556

[raulk]

Let me be more precise:

Yeah, if we're seeking a balancing effect here it needs to be formally captured. Right now it's an unspoken truth.

This behaviour is specified in the specs (libp2p/specs#219). If this is deliberate, we should explain the motivation.

[aschmahmann]

gossipsub-js seems to not impose an upper limit on gossip:

@raulk I'm confused. The line right before the ones you linked to limits the gossip to constants.GossipSubD

[vyzo]

Let's fix the naming of queueGossip; the old name was correct, as pushing to a queue is synonymous with enqueuing in common programming language lingo.
Regardless, if you are confused by push, call it enqueue, not queue.

[vyzo]

Great, merging.