libp2p-floodsub implementation doesn't conform with pubsub spec leading to incompatibilities

Summary

Hey, I was trying to integrate floodsub between rust and go node and I couldn't see the messages being received on the rust side.

After some digging, I found that (most of) the rust implementation is 7y old and likely wasn't aligned with the spec when it was being created / changed. Rust's implementation uses OneShotHandler, closing each stream after writing / reading from it, while the spec (and go impl) specifies that the communication should happen using two long-lived unidirectional streams.

Closing the stream after reading a message from the go node makes it think we dropped a connection, and make the rust node marked as dead and removed. This happens a few times (on each new connection go node sends its subscriptions) before no more retries are made or some backoff kicks in.

Here's a minimal example showing that behaviour: https://github.com/zvolin/rust-go-libp2p-floodsub-incompatibility-repro

Expected behavior

seeing messages received on rust side

Actual behavior

❯ cargo run -- /ip4/127.0.0.1/tcp/36911/p2p/12D3KooWD16BxoGs6RWmwyfiUobxw2gENLgHfUMHi7cNM1XQnQyF
   Compiling rust v0.1.0 (/home/zwolin/data/eiger/lumina/libp2p-floodsub/rust)
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 2.88s
     Running `target/debug/rust /ip4/127.0.0.1/tcp/36911/p2p/12D3KooWD16BxoGs6RWmwyfiUobxw2gENLgHfUMHi7cNM1XQnQyF`
Dialed /ip4/127.0.0.1/tcp/36911/p2p/12D3KooWD16BxoGs6RWmwyfiUobxw2gENLgHfUMHi7cNM1XQnQyF
Listening on /ip4/127.0.0.1/tcp/38103
Listening on /ip4/192.168.0.186/tcp/38103
Listening on /ip4/172.17.0.1/tcp/38103
Subscribed { peer_id: PeerId("12D3KooWD16BxoGs6RWmwyfiUobxw2gENLgHfUMHi7cNM1XQnQyF"), topic: Topic("abc") }
Subscribed { peer_id: PeerId("12D3KooWD16BxoGs6RWmwyfiUobxw2gENLgHfUMHi7cNM1XQnQyF"), topic: Topic("abc") }
Subscribed { peer_id: PeerId("12D3KooWD16BxoGs6RWmwyfiUobxw2gENLgHfUMHi7cNM1XQnQyF"), topic: Topic("abc") }
Subscribed { peer_id: PeerId("12D3KooWD16BxoGs6RWmwyfiUobxw2gENLgHfUMHi7cNM1XQnQyF"), topic: Topic("abc") }
Subscribed { peer_id: PeerId("12D3KooWD16BxoGs6RWmwyfiUobxw2gENLgHfUMHi7cNM1XQnQyF"), topic: Topic("abc") }

Possible Solution

I think that ideal solution would be to follow the go implementation architecture here, that is to have a single pubsub implementation and use some Router trait to switch its behaviour between gossiping / flooding. libp2p-gossipsub seems to be well written, maintained and conformant with the spec, so maybe the routing bits could be extracted to a separate struct, and then create another one to have flood routing.

There's also a less invasive way that could just make floodsub using long-lived unidirectional streams. Probably resulting in code duplication, but with much less changes.

I'm a fan of the first idea, but I think it may be worth it to start with just floodsub compatibility, as I believe the ideal way forward is going to take time. I'm gonna dig into this topic for few next days, maybe I'll come up with some changes.

P.S. is anyone aware of any users of the current floodsub implementation? The changes will likely be very breaking

Would you like to work on fixing this bug?

Yes

Start with the Rust floodsub implementation and its OneShotHandler stream lifecycle, then compare it with the pubsub stream-management specification and the Go implementation's comm.go behavior. Use the linked rust-go-libp2p-floodsub-incompatibility-repro to reproduce the incompatibility. Done means Rust and Go floodsub nodes maintain compatible long-lived streams and Rust receives published messages.