Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Transport initialization updates #585

Merged
merged 9 commits into from
Dec 9, 2022
Merged

Conversation

pavel-kokolemin
Copy link
Contributor

This is continuation of this discussion:
#571 (comment)

New associated type (Transport) is added to NetworkingService trait.
Now when a P2P subsystem is created a transport object is required.
For Libp2pService it's transport::Boxed<(PeerId, StreamMuxerBox)> from libp2p (with some auxiliary key details).
For MockService it's actual TransportSocket trait implementation.

I added new make_transport function to the MakeTestAddress trait to be able to create new transport in tests.

There was a problem with circular dev dependency when I tried to use updated MakeTestAddress from unit tests.
It because p2p unit tests are refering p2p-test-utils, while p2p-test-utils is referring p2p itself.
Rust does not allow build cycles so p2p is compiled twice, once with the test flag enabled and once without, making two unrelated artifacts.
And because MakeTestAddress was returning two different types (from "p2p as lib" and "p2p as test lib") compilation failed.
Here is an example of such an error: rust-lang/cargo#6765
As a workaround I moved MakeTestAddress to p2p sources.
Perhaps the proper fix would be to move unit tests from p2p to backend-test-suite instead.

Copy link
Collaborator

@TheQuantumPhysicist TheQuantumPhysicist left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good. Please address the few issues I mentioned then we can merge.

p2p/src/lib.rs Outdated Show resolved Hide resolved
Fix problem with different noise keys used in bind and connect.
Add make_transport() to MakeTestAddress trait
Move MakeTestAddress from p2p-test-utils to p2p to break circular dev dependency
Replace most TransportSocket::new calls with MakeTestAddress::make_transport()
p2p/src/testing_utils.rs Outdated Show resolved Hide resolved
p2p/src/testing_utils.rs Outdated Show resolved Hide resolved
@pavel-kokolemin pavel-kokolemin merged commit 643aeb6 into master Dec 9, 2022
@pavel-kokolemin pavel-kokolemin deleted the transport_init_updates branch December 9, 2022 13:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants