Should we always emit an initial reachability event?
Nobody has claimed this yet.
- Dominant language
- Go
- Stars
- 6.9k
- Forks
- 1.3k
- Avg merge
- 13d 21h
- Merged PRs (30d)
- 1
Description
We [currently](https://github.com/libp2p/go-libp2p/blob/master/p2p/host/autonat/autonat.go#L101) only emit an initial reachability event when we set some `forceReachability`. Normally we wait until we have some new information before emitting the our first reachability event, thus assuming if there's no event then we are in the "unknown" state.
This assumption of initial state complicates users of this event stream. Take for example [this AutoRelay code](https://github.com/libp2p/go-libp2p/blob/master/p2p/host/autorelay/autorelay.go#L80-L118). It waits for a reachability event before doing anything. Arguably It should start the `relayFinder` with our initial unknown reachability. As is, this will wait until we transition to private to start the relayfinder. This is likely a bug here (although in practice may not be too obvious).
To fix this bug with autonat's current reachability behavior is quite tricky. You want to handle two cases:
1. If no event is on the bus, assume we are in the unknown state. But we have to use some heuristic to know if there is no event (e.g. timeout).
2. If there is an event on the bus use that information (e.g. we force public reachability)
The fix for autorelay would look something like [this](https://gist.github.com/MarcoPolo/db8f4923c86eba1372c38e821a4abd83). It's pretty gross.
On the other hand, you could argue that we don't want to start the relayfinder initially at all until we observe we are private in order to avoid starting the relay finder in case we are public. I'd argue for explicitly defining that in the autorelay code rather than relying on the default unknown assumption. Another thing to keep in mind is that if AutoNat only receives "unknown reachability" observations, we will never start the relay finder.
Contributor guide
No contributing guide indexed for this repository
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Read p2p/host/autonat/autonat.go around the initial reachability event and p2p/host/autorelay/autorelay.go around relayFinder startup. Compare the current event-stream behavior with the two alternatives described in the issue, then check any related tests or entry points before deciding on the contract. Done means the initial reachability state and AutoRelay startup behavior are explicitly defined and covered by validation.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go
- Domain
- networking
- Issue type
- Bug
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Stale
- Clarity
- Needs clarification
- Newbie friendliness
- 28/100