Track more info for observed addresses

[cannium]

Fix #416

[cannium]

ping @vyzo @Stebalien for review.
Add internal address and connection direction to observed address info

[vyzo]

is this actually useful?

[cannium]

This patch pave the way for #421, so peers would see less addresses that are actually not reachable.

[Stebalien]

is this actually useful?

Very. Without this, we don't know:

1. The direction of the mapping. This matters on many NATs.
2. The actual mapping. That is, which internal address maps to which external address. Currently, we only record our external addresses but we don't actually remember which internal addresses they appear to map to.

[Stebalien]

I believe the we need to map internal to external to observation. This currently records the first observed internal address (and the first observed direction) but that may not be consistent. That is, I believe we'd want something like:

map[<internal address>]map[<observed/external address>]Observation { SeenBy: map[<peer addr>] {time, direction}}

That way, for each internal address, we can list out all the observed external addresses and figure out where from and how they've been observed.

[cannium]

I think an internal address could be observed as multiple external addresses, not the opposite. Say my node is listening on 10.0.0.1:4001, I can configure multiple port mappings for it, so observed address could be 200.0.0.1:5000, 200.0.0.1:6000, etc.

[Stebalien]

You can get both (technically). These observed addresses include observed addresses from outbound connections. It's possible for two sequential (non-overlapping) connections from different internal ports to end up using the same external port.

[cannium]

That's possible, but from my experience, is very rare. A reasonable NAT implementation would avoid port reuse in a very short time, so if two internal ports are mapped to the same external port, it's highly possible that the first connection is considered as closed.

[Stebalien]

It's also possible for a peer to lie. IMO, that's the real issue here. The only thing we know is the internal address. The external address is just an observation being reported by a peer.

[cannium]

That's still a 1:n relationship between internal address and external addresses, the map would be like external address -> internal address, or you mean we should do internal address -> list of external addresses?

[Stebalien]

The latter. Specifically: internal -> {external -> { observer -> [time, direction] } }. That way, for any port I'm explicitly listening on, I can see which external addresses have been reported. For each external address, I can see who reported them, when, and the connection direction.

[cannium]

I think I misunderstood your intention, updated now. Another idea occurs to me that ObservedAddrSet might be better tracked together with peerstore.

[cannium]

Any comments? Still invalid implementation?

[Stebalien]

Two small nits but this otherwise looks like a great improvement!

[Stebalien]

Any reason not to update oas.addrs directly?

[cannium]

Re-run CI, tests passed now.