Relay v1 error codes: 2xx vs 3xx distinction

[vyzo]

The current protocol specifies two families of 2xx and 3xx error codes for address error:

    HOP_SRC_ADDR_TOO_LONG      = 220;
    HOP_DST_ADDR_TOO_LONG      = 221;
    HOP_SRC_MULTIADDR_INVALID  = 250;
    HOP_DST_MULTIADDR_INVALID  = 251;

    STOP_SRC_ADDR_TOO_LONG     = 320;
    STOP_DST_ADDR_TOO_LONG     = 321;
    STOP_SRC_MULTIADDR_INVALID = 350;
    STOP_DST_MULTIADDR_INVALID = 351;

Why is the distinction between HOP and STOP address errors important and even desirable?
It is arguably the same error, and shouldn't matter which hop it originated from.

[vyzo]

Note that I am using the deduplicating prefixes from #24 for the error codes.

[dryajov]

Reasons we might want this:

• Allows differentiating what failed - HOP vs STOP
• Easier debuggability

Reasons we might not want this:

• Breaks under multihop dialing - i.e. no way of telling which HOP failed
• Gives out too much information to attackers as to what failed, which can be exploited with a DDoS
  • For example, find a passive node that doesn't have a connection to a peer, keep dialing the peer over the relay untill the relay locks up. Rationale - the lesser the info you expose the harder it is to identify an attack surface.

[vyzo]

It is arguable whether there is any benefit to debugging in distinguishing the host that failed the address error check.

On the negative side, it also adds implementation complexity: implementations need different error reporting paths for their address checking code in order to report the correct error code.

And there is also the conceptual complexity, as we have 4 otherwise identical error codes we have to explain their semantics and justify their existence (I am a big fan of minimality in protocol specification).

[dryajov]

It is arguable whether there is any benefit to debugging in distinguishing the host that failed the address error check.

I tend to agree with this point, does it matter which host failed the address check? As long as its caught and reported back to the src it should be sufficient to properly handle that path.

I need to check the spec again, but I don't think the HOP modifies the addresses it got from src in any way.

[dryajov]

@diasdavid @whyrusleeping @lgierth wanna chime in on this one?

[whyrusleeping]

I think i'm okay with not differentiating between address too long and invalid multiaddr. And i agree with @vyzo that we probably dont need to distinguish between this error coming from hop or stop, you should generally be able to infer this from the context in which you received the error. We can probably drop these down to just two error codes?

[dryajov]

@whyrusleeping 👍 on dropping error codes.

[daviddias]

@whyrusleeping 👍

[mxinden]

@vyzo do I understand correctly that circuit relay v2 addresses this issue offering neither of the Status above?

https://github.com/libp2p/go-libp2p-circuit/blob/90d67900a8fe277fb629b29f09d0fe8efbe40aaf/v2/pb/circuit.proto#L51-L60