rust-libp2p: gossipsub: `multi_hop_propagation` test failed in CI

Summary

See https://github.com/libp2p/rust-libp2p/actions/runs/3652352144/jobs/6170648691#step:10:673.

test multi_hop_propagation ... FAILED

failures:

error: test failed, to rerun pass `-p libp2p-gossipsub --test smoke`
---- multi_hop_propagation stdout ----
thread 'multi_hop_propagation' panicked at '[quickcheck] TEST FAILED (runtime error). Arguments: (44, 3353857924130305067)
Error: Timed out waiting for all nodes to receive the msg but only have 0/44.', /home/runner/.cargo/registry/src/github.com-1ecc6299db9ec823/quickcheck-1.0.3/src/tester.rs:165:28
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

Expected behaviour

Actual behaviour

Debug Output

<output>

Possible Solution

Version

libp2p version (version number, commit, or branch):

Would you like to work on fixing this bug?

No.

About this issue

Original URL
State: open
Created 2 years ago
Comments: 18 (13 by maintainers)

Most upvoted comments

Thanks so much @AgeManning for all the help and all the explanation, it’s way more clear to me now, it sounds like I was looking to use the protocol in the wrong way.

bochaco on Oct 5, 2023

Hey @bochaco.

I’m not sure about the bug you are referring to with flood publishing. We have tests to ensure flood publishing works. See for example here: https://github.com/libp2p/rust-libp2p/blob/master/protocols/gossipsub/src/behaviour/tests.rs#L2105

I’m not really following your first commit. Instead of collecting all the peers on a given topic (the set variable), you now collect all peers on all topics. Unless there is not a single peer subscribed to a single topic, this set will never be empty. And then you use that set to send messages to all the peers in the event that we do not have flood_publish enabled.

I think this commit will then send to all peers when flood publish is not enabled, even those that are not subscribed to the topic.

Your new test, i imagine would fail often. You setting up 50-100 peers and having only one subscribe to the topic and trying to get messages sent among the nodes. So yeah, this would definately come down to topology.

The third thing you have added is also not desired I dont think. We shouldn’t be sent messages to topics we are not subscribed to. So in any functional environment we shouldn’t receive these messages. If one is sent to us by mistake, we shouldn’t propagate it onwards, because its an error and potentially a DOS vector.

I’ll comment further on your PR.

AgeManning on Oct 4, 2023

we probably have to make it deterministic.

I guess this would require us to parameterize the RNG source for the entire behaviour?

thomaseizinger on Oct 3, 2023