libp2p
Add identify protocol to test-plans
Ideally our Testground tests would catch issues like this in the future. Given that our current Testground tests run the libp2p-ping protocol only, they have not caught it.
See: https://github.com/libp2p/rust-libp2p/issues/3244
Unless this happens organically, this is probably difficult to write a reliable test case for. We’d need to have one implementation generate a multiaddr that another implementation doesn’t understand. An Identify implementation most likely doesn’t have an API to inject invalid addresses.
It sounds like this would be a good thing to test in a unit test though.
This particular issue could be caught if our test binaries would listen all interfaces they support. A node supporting quic would then return that in its identify payload, even if the test says use TCP as the transport. Running this against a node that doesn't speak QUIC would have caught this problem.
Would it be a good idea to always enable support for everything protocol we have on the listener side and only use the env variable on the dialer side to limit support for a specific stack?
There’s two types of „unsupported addresses“:
- addresses of transports that you’re not listening on
- addresses containing multiaddress components you don’t know about. This happens when we define new transports in the future.
(1) is easy to test, (2) is more challenging. If I understand correctly, the nature of the failure in https://github.com/libp2p/rust-libp2p/issues/3244 was of the second nature.
Correct. If a go node listens on QUIC and TCP and is paired with a Rust v0.49.0 node (doesn't support QUIC) to test the TCP + yamux setup, we would have noticed that an incoming identify payload with a QUIC address in it causes problems.
That’s more of a coincidence though. We have no way of testing this reliably, i.e. test that the most recent version of rust-libp2p (or go-libp2p, for that matter), doesn’t stumble over this (unless we add another transport with its own multiaddr).
To be clear, having Identify tests is useful anyway, and I think we should add a test plan. However, there are failure modes that are easy to test in interop testing and other that are not.