AutoNAT: return E_DIAL_REFUSED instead of E_DIAL_ERROR of when skipping dial

[elenaf9]

Discussed in libp2p/specs#411:
When skipping a dial as a server, we should return E_DIAL_REFUSED instead of E_DIAL_ERROR because we never actually tried to dial the client.

[MarcoPolo]

I think we should leave a comment here too: https://github.com/libp2p/go-libp2p/blob/master/p2p/host/autonat/client.go#L79 since it's used there by the client.

[elenaf9]

I think we should leave a comment here too: https://github.com/libp2p/go-libp2p/blob/master/p2p/host/autonat/client.go#L79 since it's used there by the client.

But there it only checks if there is an error, not what type of error it is. Shouldn't we then rather put that comment on the DialBack function itself, or here:

go-libp2p/p2p/host/autonat/autonat.go

Lines 376 to 396 in f383f46

	a, err := cli.DialBack(ctx, pi.ID)
	var result autoNATResult
	switch {
	case err == nil:
	log.Debugf("Dialback through %s successful; public address is %s", pi.ID.Pretty(), a.String())
	result.Reachability = network.ReachabilityPublic
	result.address = a
	case IsDialError(err):
	log.Debugf("Dialback through %s failed", pi.ID.Pretty())
	result.Reachability = network.ReachabilityPrivate
	default:
	result.Reachability = network.ReachabilityUnknown
	}
	select {
	case as.observations <- result:
	case <-as.ctx.Done():
	return
	}
	}

where the type of error is checked?

[MarcoPolo]

But there it only checks if there is an error, not what type of error it is. Shouldn't we then rather put that comment on the DialBack function itself, or here:

…
where the type of error is checked?

I think that makes sense. I’d like to cover the case where someone in the future looks at this code and thinks “Hmm, if I check for dial refused I can do something clever” and doesn’t know that older peers will almost always return dial error. As long as that case is covered, I’m happy :)

[marten-seemann]

@elenaf9 Are you planning to address @MarcoPolo's comments? Would be great if we could this fix into the next release!

[elenaf9]

@elenaf9 Are you planning to address @MarcoPolo's comments? Would be great if we could this fix into the next release!

Yes, will add something later today or tomorrow. Sorry for the delay.

[marten-seemann]

Replaced by the (rebased and squashed) #1527.