Replace peer addresses in identify

[vyzo]

Closes #579.

[vyzo]

A concern here is that we might be clobbering permanent addresses.

[vyzo]

It looks like we'll need a change in the peerstore interface to avoid clobbering them.

[vyzo]

An approach that doesn't require a change is to set the ConnectedAddrTTL address to be TempAddrTTL with UpdateAddrs and then do AddAddrs for the new address with ConnectedAddrTTL.
A bit hacky, but it might just work.

[vyzo]

In terms of interface changes, we can perhaps add GetAddrs(p, ttl) call to peerstore.
This way we can do GetAddrs(p, PermenentTTL) before the SetAddrs and then re-add the permenent addresses.

This would look like this:

	permaddrs := id.Host.Peerstore().GetAddrs(p, pstore.PermenentAddrTTL)
	switch ids.Host.Network().Connectedness(p) {
	case inet.Connected:
		ids.Host.Peerstore().SetAddrs(p, lmaddrs, pstore.ConnectedAddrTTL)
	default:
		ids.Host.Peerstore().SetAddrs(p, lmaddrs, pstore.RecentlyConnectedAddrTTL)
	}
	if len(permaddrs) > 0 {
		id.Host.Peerstore.AddAddrs(p, permaddrs, pstore.PermenentAddrTTL)
	}

thoughts @Stebalien @raulk ?

[raulk]

Relates to libp2p/go-libp2p-identify#2 cc @hsanjuan.

[Stebalien]

This function is really deceptive. It sets the TTLs for the given addrs but doesn't actually swap out the old addrs for a new set.

As far as I can tell, nobody actually uses this. We now use UpdateAddrs to atomically change TTLs.

[vyzo]

I don't think UpdateAddrs is sufficient here: it only updates the given addrs and requires us to know the old TTL. It only changes the TTLs, it doesn't remove addrs.
Specifically, if the most recent IdentifyPush removed addrs from the set, ie maybe because autorelay has rewritten the addr set, then the old addrs not in the new set will stick around forever!

[vyzo]

My proposal is to modify SetAddrs to discard the old addrs and act as a real setter (and be its first user), and introduce a new GetAddrs method that allows us to retrieve the old addrs associated with some TTL. The latter will allow us to avoid clobbering permanent Addrs.

[vyzo]

Actually, there might be a way to do the dance with UpdateAddrs and AddAddrs.
First update addrs of ConnectedTTL to TempTTL and then AddAddrs with connected ttl.

It doesn't have the same effect as removing addrs, but it's close as it will mark them eligible for garbage collection and reduce the exposure time for gossiping invalid addrs.

[vyzo]

We can go one step further and set the TTL to 0 to invalidate them, no interface change necessary.

[vyzo]

Fixed and rebased/squased//force-pushed.

[hsanjuan]

My interest here boils down to never deleting or changing the TTL on addresses added with PermanentAddrTTL, even if they don't work.

[vyzo]

The implementation in 06391d4 never touches permanent addr TTLs.

It only replaces addresses with ConnectedAddrTTL.

[Stebalien]

I wasn't actually suggesting that we use UpdateAddrs (I was also thinking about fixing SetAddrs) but I actually like this more.

Maybe we should downgrade these to TempAddrTTL instead of 0? Honestly, I'm fine either way and TempAddrTTL is long enough that that might cause problems. My only concern is that we'll have a short period where we'll have no good addresses.

We could also use a constant 10s timeout (for really temporary addresses).

[vyzo]

It's a valid concern I think, so I'll update to use a 10s ttl to cover the gap.

[vyzo]

I called it transientTTL and it's 10s long; maybe we should have this constant in the peerstore.

[Stebalien]

Well, that's what TempAddrTTL was supposed to be...