Add implementation feature to protocol id

[emhane]

Description

Adds implementation feature to the protocol ID.

Notes & open questions

Change checklist

• Self-review
• Documentation updates if relevant
• Tests if relevant

[AgeManning]

I'm not sure we want to mess with the protocol-id. I still want to remove it altogether at some point: #195

I think if we mess with the protocol-id then we are really going off-spec and will only be able to communicate with ourselves.

Also I think this approach will segregate client versions. I.e only nodes that support this specific version will be able to communicate with each other. For the Ethereum network, we very often have a wide variety of client implementations and versions on the network and we would always want to communicate with these peers.

I think the approach that keeps this implementation fully backwards-compatible and to-spec with 5.1 is the way to go.

[emhane]

I'm not sure we want to mess with the protocol-id. I still want to remove it altogether at some point: #195

ah, ok, I think it's neat actually. but it could have stayed a generic on the Handler struct like before.

I think if we mess with the protocol-id then we are really going off-spec and will only be able to communicate with ourselves.

Also I think this approach will segregate client versions. I.e only nodes that support this specific version will be able to communicate with each other. For the Ethereum network, we very often have a wide variety of client implementations and versions on the network and we would always want to communicate with these peers.

I think the approach that keeps this implementation fully backwards-compatible and to-spec with 5.1 is the way to go.

exactly, the pr doesn't change the version, but adds "implementation features" to the type. could also be called "pre-release discv5.2 features", that would fit.

the new messages introduced for hole punching are contained in the new PacketKind SessionMessage https://github.com/emhane/discv5/blob/e3478c2d7c4021b2eeb2e88be61ba9ccd2297cd5/src/packet/mod.rs#L287-L300, so filtering out early is easy

wether to store a peer as new_peer_latest_relay_cache must be checked against their enr, if the peer has a nat field set. at best we add that field to the session supports_nat_feature: bool, and set it upon session establishment, so that it is quickly accessible for Handler when deciding what to do upon (i) request time out and (ii) receiving nodes responses. (i) should I try hole punching the node? (ii) if there are new peers in this response, should I store the sender as latest seen relay for them?

this is not an exhaustive list, but I don't think there is much more to do than that.

[emhane]

anyhow, this version related change, isn't blocking merging hole punching into discv5.2. it blocks merging discv5.2 into main @AgeManning

[emhane]

closing in favour of ethereum/devp2p#227