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

bitswap: BitSwapNetwork notifies about connected peers that do not speak Bitswap #631

Open
Wondertan opened this issue Jun 26, 2024 · 4 comments
Assignees
Labels
dif/medium Prior experience is likely helpful effort/days Estimated to take multiple days, but less than a week kind/enhancement A net-new feature or improvement to an existing feature P2 Medium: Good to have, but can wait until someone steps up topic/bitswap

Comments

@Wondertan
Copy link
Member

Here we listen and pass newly connected peers to Bitswap Client and Server. However, we do that for all our peers, including those who do not speak Bitswap protocol.

This is not a big problem because we will quickly realize that such peers are unresponsive, but we could still filter them out much earlier. This potentially warrants migrating to Identify libp2p events that now contain the list of protocols the peer supports.

@Wondertan Wondertan added the need/triage Needs initial labeling and prioritization label Jun 26, 2024

This comment was marked as off-topic.

@lidel
Copy link
Member

lidel commented Jun 27, 2024

@Wondertan thank you for filling this, by chance, do you have a ballpark % numbers of usable vs unusable peersyou've experienced?

cc @aschmahmann @gammazero this sounds oddly familiar, could contribute to the bitswap behavior we've seen in Rainbow traces?

@Wondertan
Copy link
Member Author

We don't have any numbers yet. I realized this is an issue while exploring the code for the new protocol we are building over Bitswap. We will be running a single Server over two BitSwapNetwork and I needed to know the implications of that.

@lidel lidel added P2 Medium: Good to have, but can wait until someone steps up dif/medium Prior experience is likely helpful effort/days Estimated to take multiple days, but less than a week kind/enhancement A net-new feature or improvement to an existing feature topic/bitswap and removed need/triage Needs initial labeling and prioritization labels Jul 2, 2024
@gammazero gammazero self-assigned this Jul 29, 2024
@gammazero
Copy link
Contributor

When a peer connects, the peer's supported protocols are not immediately known. We may be able to wait for identify to complete before sending notification, and then see what protos are supported. Any messages from the peer will still cause a connected notification when the state changes to stateResponsive.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dif/medium Prior experience is likely helpful effort/days Estimated to take multiple days, but less than a week kind/enhancement A net-new feature or improvement to an existing feature P2 Medium: Good to have, but can wait until someone steps up topic/bitswap
Projects
None yet
Development

No branches or pull requests

3 participants