Gossipsub mesh contains same peer id multiple times

[rklaehn]

The mesh for a topic is a Vec.

After running a node for a while while looking into #1671, the mesh for a topic contains lots of duplicates.

    mesh: {
        TopicHash {
            hash: "discovery",
        }: [],
        TopicHash {
            hash: "/klk/prod/2020-01-09",
        }: [
            PeerId(
                "QmWR3WVgkVUNG3HEdU2xjW4wW5pcKF3AL3Aht3ixYHJVrW",
            ),
            PeerId(
                "QmWR3WVgkVUNG3HEdU2xjW4wW5pcKF3AL3Aht3ixYHJVrW",
            ),
            PeerId(
                "QmWR3WVgkVUNG3HEdU2xjW4wW5pcKF3AL3Aht3ixYHJVrW",
            ),
            PeerId(
                "QmWR3WVgkVUNG3HEdU2xjW4wW5pcKF3AL3Aht3ixYHJVrW",
            ),
            PeerId(
                "QmWR3WVgkVUNG3HEdU2xjW4wW5pcKF3AL3Aht3ixYHJVrW",
            ),
            PeerId(
                "QmWR3WVgkVUNG3HEdU2xjW4wW5pcKF3AL3Aht3ixYHJVrW",
            ),
            PeerId(
                "QmWR3WVgkVUNG3HEdU2xjW4wW5pcKF3AL3Aht3ixYHJVrW",
            ),
            PeerId(
                "QmWR3WVgkVUNG3HEdU2xjW4wW5pcKF3AL3Aht3ixYHJVrW",
            ),
            PeerId(
                "QmWR3WVgkVUNG3HEdU2xjW4wW5pcKF3AL3Aht3ixYHJVrW",
            ),
            PeerId(
                "QmWR3WVgkVUNG3HEdU2xjW4wW5pcKF3AL3Aht3ixYHJVrW",
            ),
            PeerId(
                "QmWR3WVgkVUNG3HEdU2xjW4wW5pcKF3AL3Aht3ixYHJVrW",
            ),
        ],
    },

This definitely does not seem right, since the size of the vec is being used for checking if the mesh needs more peers.

                    // if the mesh needs peers add the peer to the mesh
                    if let Some(peers) = self.mesh.get_mut(&subscription.topic_hash) {
                        if peers.len() < self.config.mesh_n_low {                        

Surely a mesh that contains > mesh_n_low times the same node is not a very good mesh.

I think the mesh peers should be a set to avoid all those issues. If it needs to be a vec for perf reasons, at least it should be ensured that the vec does not contain duplicates.

[rklaehn]

Note that this might be caused by non-spec compliant behaviour of the remote node (it is a 0.4.22 go-ipfs node). I have no idea.

But the internal state should not get inconsistent even when faced with non-spec-compliant msgs.

[rklaehn]

I think it does not grow indefinitely, but only up to mesh_n_high. So you end up with a mesh that contains mesh_n_high duplicates of the same node...

[rklaehn]

I am pretty sure this will be fixed by the switch to sets in #1583

[AgeManning]

Resolved in #1583