[swarm] Configurable and "infinite" scores for external addresses.

[romanb]

Motivation

It is currently not possible to add external addresses to a Swarm that are retained forever. All added or observed external addresses are added to an ordered list with a finite score (1) for every new addition and thus subject to being purged eventually. It is desirable to be able to add addresses to this collection that are retained until explicitly removed and always sorted at the beginning, not influencing the existing semantics for the additional finite scores and expiring addresses. It also seems desirable to let the score of a particular address report / observation be chosen freely, instead of being fixed to 1. The motivation relates to paritytech/substrate#7518.

To that end, this PR extends the address score arithmetic used for external addresses with an infinite cardinal, permitting addresses with such a score to be retained "forever" or until explicitly removed, thereby exposing address scores on the public API for more insight and configurability.

Implementation

• The score of an (external) address is now not just a u32 but an AddressScore, i.e. either AddressScore::Finite(u32) or AddressScore::Infinite.
• Only finitely scored addresses compete among each other for retention in the ordered address list, the same way as before. "Infinitely scored" addresses are always retained and sorted at the beginning, i.e. higher than any finite score.
• Swarm::add_external_address now takes both a Multiaddr and an AddressScore.
• Analogously to the above, NetworkBehaviourAction::ReportObservedAddr now also takes an AddressScore in addition to the Multiaddr. The Identify behaviour reports all addresses with a finite score of 1, i.e. the same semantics as before.
• A new method Swarm::remove_external_address for explicit removal, regardless of score. This allows for explicit curation of the external address list.
• Iterating over the external addresses now also provides access to the score via iterating over AddressRecords.
• The external addresses given to the NetworkBehaviour::poll are also accompanied by their score (again via AddressRecords).

[tomaka]

Thanks for tackling this!

In my opinion, this external addresses scoring system isn't great. It is a bit opaque, and multiple ReportObservedAddr events reported by multiple different NetworkBehaviours will compete with each other without necessarily being aware of it.

For example, the identify behaviour will generate a ReportObservedAddr every 60 seconds. If I add another behaviour that reports a ReportObservedAddr every 30 seconds, this other behaviour will implicitly have double the weight of the one generated by identify. It is very difficult to understand the behaviour of external addresses without knowing how the entirety of libp2p works.

But if we go under the constraint that we keep this scoring system, which is the best short-term solution, then this PR does make sense.

[romanb]

In my opinion, this external addresses scoring system isn't great. It is a bit opaque, and multiple ReportObservedAddr events reported by multiple different NetworkBehaviours will compete with each other without necessarily being aware of it.

I tried to improve the documentation, but I'm not sure what you mean by different behaviours competing. If they both issue a report for the same address, the effect is cumulative. For different addresses the ones that are reported with the higher cumulative score (e.g. either more frequently or with higher scores) have the higher retention and priority. That seems to make sense to me, even under composition of behaviors. Of course behaviours / protocols should make an attempt at documenting how they go about reporting addresses, like I did for identify, to better reason about the composition. Nevertheless I'm open to alternative proposals on how to handle external addresses, i.e. feel free to open a new issue describing the problems with the current approach and possible alternatives, if you already have something in mind.

But if we go under the constraint that we keep this scoring system [..]

That was my current understanding at least, so I only looked into extending it, rather than replacing it. Unless we already have a clear idea of how a replacement should look like, I would also suggest to go with this for now.

[tomaka]

Of course behaviours / protocols should make an attempt at documenting how they go about reporting addresses, like I did for identify, to better reason about the composition.

That's indeed the only solution to this problem. When I say "It is very difficult to understand the behaviour of external addresses without knowing how the entirety of libp2p works.", what I mean is that you need to look at the precise documentation of every NetworkBehaviour that you use.

It would be preferable to me, although probably difficult to achieve, that adding a network behaviour to the stack never has any negative consequence on the others.

That was my current understanding at least, so I only looked into extending it, rather than replacing it. Unless we already have a clear idea of how a replacement should look like, I would also suggest to go with this for now.

I didn't mention anything about keeping or replacing the current system because I also don't know how we could replace it. I'm fine with keeping it if there's no obvious suitable replacement.