identify: simplify implementation by removing peer tracking

[marten-seemann]

Identify currently deduplicates deltas sent to peers that we hold multiple connections to. This creates a lot of complexity in the implementation of the protocol.

It would be nice if we could simplify the logic to act on connections instead. There'd be no complicated tracking logic: start handling a connection when it's opened / accepted, and handle it until it is closed.

I measured how frequent duplicate connections really are by running Kubo both on my local machine (behind a CGNAT) and on a public server. The highest number of peers with duplicate connections I've come across was duplicates to 9 peers out of a total of 159, but that value quickly dropped as connections churned. For the vast majority of the time, we had duplicate connections to about 1% of the peers.
I expect that the number of duplicates will drop even further once we implement a smarter dialing logic (#1785), although we will never be able to completely eliminate them, given that two peers dialing each other at exactly the same time will end up with two connections.

@vyzo @Stebalien quick sanity check, does this make sense?

[Stebalien]

IIRC, this wasn't about duplicates but more about ordering. I believe a lot of this logic was trying to handle dead connections, etc.

[marten-seemann]

What about introducing a sequence number in the Delta message:

go-libp2p/p2p/protocol/identify/pb/identify.proto

Lines 5 to 10 in 8231591

	message Delta {
	// new protocols now serviced by the peer.
	repeated string added_protocols = 1;
	// protocols dropped by the peer.
	repeated string rm_protocols = 2;
	}

Senders could set it to the current timestamp (this would be completely stateless), and receivers could ignore messages where the sequence number is smaller than the last sequence number they received.

[Stebalien]

That would be the correct solution, yes. I believe the current version may have been my attempt to work around the issue without breaking network compatibility? But yes, we should just have sequence numbers. Note: we technically also need to handle skips.

[Stebalien]

Also see libp2p/specs#500. We should be able to just leave a stream open...

[marten-seemann]

The problem is what happens when we have two connections: We receive message N+2 on one connection (skipping N+1), then we receive N+1 on the other one. Now it's not correct to apply N+1 any more (only applying N+1 and N+2 sequentially would be correct). But this means that the receiver would have to buffer messages that it already applied, just in case a message was skipped.