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

Introduce connected_peers() API #4400

Closed

Conversation

shamil-gadelshin
Copy link
Contributor

This PR adds connected_peers API to Substrate Network (sc-network crate). The new API exports a list of the currently connected peers. It's helpful when we need to issue custom requests (libp2p request-response protocol) to some peer because the upstream protocol uses PeerId to target a specific peer. Actually, request-response protocol is able to initiate a new connection when we pass PeerId (it tries to obtain the address) but for this use-case we expose currently connected peers only.

@bkchr
Copy link
Member

bkchr commented May 10, 2024

I don't get your explanation. If you only want to start_request to currently connected peers, you should use IfDisconnected::ImmediateError.

@nazar-pc
Copy link
Contributor

I don't get your explanation. If you only want to start_request to currently connected peers, you should use IfDisconnected::ImmediateError.

But how do you know what peers node is connected to the first place?

@bkchr
Copy link
Member

bkchr commented May 12, 2024

But how do you know what peers node is connected to the first place?

Why do you need to know? Just send to all the peers you are interested in? Also your protocol should know which peers are connected via this protocol?

@nazar-pc
Copy link
Contributor

We're using it to implement #4407, Shamil should be able to provide more details once he is back online tomorrow.

@bkchr
Copy link
Member

bkchr commented May 12, 2024

Then you could directly use this?

@shamil-gadelshin
Copy link
Contributor Author

Then you could directly use this?

The link doesn't work, however, it seems you're suggesting peers_info API from syncing_engine. Although it provides networking info indirectly it might work even better because of the additional info provided (peer roles). Thanks! The downside is that the API may change with changing of the container module - syncing_engine.

Let me test that it works as expected.

@shamil-gadelshin shamil-gadelshin marked this pull request as draft May 13, 2024 07:12
@shamil-gadelshin
Copy link
Contributor Author

peer_info API returns times less peers than connected_peers in our setup. It's likely because of the multiple peer sets that networking crates manage compared to the syncing engine. However, the provided data is enough for our use case.

Thanks for the review!

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

3 participants