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

feat: Add Identify + Kademlia chat example #3150

Closed
wants to merge 1 commit into from

Conversation

mvdbos
Copy link

@mvdbos mvdbos commented Nov 21, 2022

Description

The kad-identify-chat example implements simple chat functionality using the Identify protocol
and the Kademlia DHT for peer discovery and routing. Broadcast messages are propagated using the
Gossipsub behaviour, direct messages are sent using the RequestResponse behaviour.

The primary purpose of this example is to demonstrate how these behaviours interact, as Identify and Kademlia are not used together in any other example.

A secondary purpose of this example is to show what integration of libp2p in a complete
application might look like. This part is similar to the file sharing example,
but where that example is purposely more barebones, this example is a bit more expansive and
uses common crates like thiserror and anyhow. It also uses the tokio runtime instead of async_std.

Notes

The reason for adding a new example is that I could not find an example that showed how to use Identify with Kademlia for peer discovery and routing. I realize the example is on the big side, and I have continuously tried to balance between realism and simplicity. Let me know if you see areas where this can be simplified, without compromising the realism.

Open Questions

Change checklist

  • I have performed a self-review of my own code
  • I have made corresponding changes to the documentation
  • N/A I have added tests that prove my fix is effective or that my feature works
  • N/A A changelog entry has been made in the appropriate crates

@mvdbos mvdbos changed the title Add Identify + Kademlia chat example feat: Add Identify + Kademlia chat example Nov 23, 2022
@mvdbos
Copy link
Author

mvdbos commented Nov 24, 2022

@thomaseizinger @mxinden @elenaf9 Do you think this example should be added?

This is a bigger example application that uses the
Identify and Kademlia behaviours for peer discovery.

For the chat functionality, it uses Gossipsub and
RequestResponse behaviours.

- `identify::Behaviour`: when peers connect and they
  both support this protocol, they exchange `IdentityInfo`.
  `MyChatNetworkBehaviour` then uses this info to add their
  listen addresses to the Kademlia DHT.
  Without `identify::Behaviour`, the DHT would propagate
  the peer ids of peers, but not their listen addresses,
  making it impossible for peers to connect to them.
- `Kademlia`: this is the Distributed Hash Table implementation,
  primarily used to distribute info about other peers on the
  network this peer knows about to connected peers. This is a core
  feature of `Kademlia` that triggers automatically.
  This enables so-called DHT-routing, which enables peers to send
  messages to peers they are not directly connected to.
- `Gossipsub`: this takes care of sending pubsub messages to all
  peers this peer is aware of on a certain topic.
  Subscribe/unsubscribe messages are also propagated.
  This works well in combination with `identify::Behaviour`
  and `Kademlia`, because they ensure that Gossipsub messages
  not only reach directly connected peers, but all peers that can
  be reached through the DHT routing.
- `RequestResponse`: `MyChatMessageExchangeCodec` is a simple
  request response protocol, where received messages will
  automatically be responded to with a 0 byte. This confirms
  to the sender that the message was correctly received.
  Consider that analogous to the 'delivered' check on WhatsApp/Signal.
  This can only be used to send message to directly connected peers.
  It does not use the DHT routing. It expect the local peer to know
  the listen address of the peer it is sending a message to.
  If we expect peers to not always be able to directly reach
  other peers because of networking reasons, we may have
  to add the `Relay` behaviour to `MyChatNetworkBehaviour`
@mvdbos mvdbos force-pushed the feature/kad-identify-example branch from 9df0afa to 9f7312f Compare November 24, 2022 12:45
@dariusc93
Copy link
Member

The way i would generally discover peers with DHT is using start_providing and get_providers. In case of pubsub, you could provide the topic over DHT using start_providing and in an interval use get_providers (though to my knowledge, #2712 improves on this so polling a timer may not be needed but have to read it over to get a better understanding). I think IPFS does this as well by storing the topic as a block and providing the cid for their pubsub but dont quote me on that (although this pr isnt related specifically to ipfs). Just some thoughts i would give about using kademlia identify and gossipsub. I do have an example https://github.com/dariusc93/libp2p-chat-test (although not updated to using the latest libp2p) utilizes providers for peer discovery

Copy link
Member

@mxinden mxinden left a comment

Choose a reason for hiding this comment

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

Wow, thanks for the extensive contribution.

I am a bit undecided on whether we should merge it though. Given that it is such an extensive example, I expect that it is either outdated soon, or requires a lot of maintenance.

Instead, I would prefer:

What do you think @mvdbos? Would you be open to contributing many smaller patches to the file sharing example which we can then discuss in sequence.

While we might not merge here, I think this is great material. Have you considered publishing this as a blog post?

@mvdbos
Copy link
Author

mvdbos commented Dec 4, 2022

@mxinden TL;DR: happy to contribute in another form, but first I will give it one more try to convince you to merge it:

I am a bit undecided on whether we should merge it though. Given that it is such an extensive example, I expect that it is either outdated soon, or requires a lot of maintenance.

I fully understand your concern about maintainability and your reluctance to add another bigger example. To offset that, there must be a strong enough reason to introduce this maintenance burden. I believe there is enough value in an additional real-world example, since it covers a very different use case (chat): it is meant as a natural progression from the simpler chat examples. If found that even after referencing the file-sharing example, I had to do quite a bit of digging to find out how to connect the NetworkBehaviours. It also shows a different use for the RequestResponse behaviour.

On the risk of becoming outdated: while there is a decent amount of code, most of it is quite isolated from rust-libp2p. Other than that, it uses very common crates such as serde, thiserror, etc.

For the "showcase how identify and Kademlia can work together", fix this problem at the root by implementing #2680.

Agree that fixing the root cause is best. But given that #2680 is already pending since May, and given that it seems hooking up identify to kad in this way seems the only way until then, does it not merit at least a simple example until then?

What do you think @mvdbos? Would you be open to contributing many smaller patches to the file sharing example which we can then discuss in sequence.

I would if I can't convince you to add the whole thing. Which parts did you have in mind? Only the Identify + Kad, or more? Not all is relevant to file-sharing. There is also a bit of overlap on how event handling is organized.

While we might not merge here, I think this is great material. Have you considered publishing this as a blog post?

I have. I was actually hoping to merge this and then write a blog post about it. :-D

@thomaseizinger
Copy link
Contributor

I am happy to merge a bigger example if we:

Ideally, I'd like all our examples to be orthogonal to each other. I think examples of different levels of detail are not that useful, esp. if they don't represent what we'd consider idiomatic rust-libp2p usage.

With #3068 merged, using a different executor is quite foolproof I'd say so we don't necessarily need an entire example for that.

@mvdbos
Copy link
Author

mvdbos commented Dec 12, 2022

Thanks @thomaseizinger and @mxinden for the feedback. And apologies for the late reaction.

Perhaps I misunderstand, but I am not sure if both your reactions align when it comes to nex steps for this specific PR. :-) Should I turn it in to smaller contributions to the file sharing example as @mxinden suggests? Or move it as @thomaseizinger suggests, and also delete some other examples related to Identify and Tokio?

Regardless, I believe it is justified to add an example now on how to get Identify to work with the DHT, since it may be a long time before #2680 is merged (it is open since May). Do you agree? If so, I am happy to reduce this example to a much smaller one that only illustrates the Identify + Kademlia logic, or add that logic to the file sharing example.

if they don't represent what we'd consider idiomatic rust-libp2p usage.

@thomaseizinger Do you refer to this PR? I am not aware of any non-idiomacy, but that is probably me. Can you elaborate?

@thomaseizinger
Copy link
Contributor

Thanks @thomaseizinger and @mxinden for the feedback. And apologies for the late reaction.

Perhaps I misunderstand, but I am not sure if both your reactions align when it comes to nex steps for this specific PR. :-)

I don't think the next steps are necessarily decided already! @mxinden agreed to the general direction of #3111. Would you be up for working on that? As part of that, the existing examples can be cleaned up and some of them consolidated / extended such that we also demonstrate the identify + kademlia interaction.

Regardless, I believe it is justified to add an example now on how to get Identify to work with the DHT, since it may be a long time before #2680 is merged (it is open since May). Do you agree? If so, I am happy to reduce this example to a much smaller one that only illustrates the Identify + Kademlia logic, or add that logic to the file sharing example.

if they don't represent what we'd consider idiomatic rust-libp2p usage.

@thomaseizinger Do you refer to this PR? I am not aware of any non-idiomacy, but that is probably me. Can you elaborate?

I was talking about our examples in general. They could use a lot of work but it is tricky to find the time for it.

@mvdbos
Copy link
Author

mvdbos commented Dec 14, 2022

[...] the general direction of #3111. Would you be up for working on that?

@thomaseizinger: I would, but I have a few remarks/questions:

  • Before we start on Move all examples to a common location #3111 and especially the consolidation of existing examples, I think we should agree on what exactly we want to demonstrate. I am not talking about tech designs, but about what we consider the canonical examples (file sharing, chat, etc.) These could then indeed be 'skeleton' crates for users to start from. There should probably not be more than 4-5 of these and they should probably be more like full app skeletons than simple examples. These could then also be covered in accompanying tutorials on the doc site.
  • When focusing on the fuller examples, we may not be able to cover all of the current minor examples. Could it be an option to keep some of these around and move them to their respective sub-crates? This goes against your proposal: it would mean an example is either a full skeleton in a separate binary, or a minor component example, part of its own crate. So the central examples still go away.
  • When creating these fuller examples/skeletons, I expect a pattern will emerge on how to handle event loops etc., as can already be seen in this PR and in the file sharing example. Perhaps we should form an opinion on what that should look like to ensure consistency, or even consider introducing some more high-level reusable components for common patterns such as event handling.

I was talking about our examples in general. They could use a lot of work but it is tricky to find the time for it.

I would love to focus on this full-time, but I still have another day job... :-)

@mergify
Copy link
Contributor

mergify bot commented Dec 19, 2022

This pull request has merge conflicts. Could you please resolve them @mvdbos? 🙏

@thomaseizinger
Copy link
Contributor

When creating these fuller examples/skeletons, I expect a pattern will emerge on how to handle event loops etc., as can already be seen in this PR and in the file sharing example. Perhaps we should form an opinion on what that should look like to ensure consistency, or even consider introducing some more high-level reusable components for common patterns such as event handling.

Yes, that is exactly one of the things I am looking forward to. The idea was to have this discussion as part of doing the issue but we can discuss it somewhere separately first as well if you prefer!

Before we start on #3111 and especially the consolidation of existing examples, I think we should agree on what exactly we want to demonstrate. I am not talking about tech designs, but about what we consider the canonical examples (file sharing, chat, etc.) These could then indeed be 'skeleton' crates for users to start from. There should probably not be more than 4-5 of these and they should probably be more like full app skeletons than simple examples. These could then also be covered in accompanying tutorials on the doc site.

Yep, agree with you here too. I think there are at least two bigger categories of apps (in terms of architecture):

  1. "True" p2p nodes where each nodes has the same functionality. These are the most likely to use a DHT for discovery. Kademlia is typically paired with identify as well. We can add mDNS into this and also ping to keep connections alive. Likely these nodes also use gossipsub to send data across the network.
  2. Apps which have more explicit client-server roles. For example relays, rendezvous etc.

Following from this, I think we should have (at least) 4 different examples:

  1. A p2p example, usecase to be decided but it chat is probably a useful one. Note that I'd also like this example to have a "bootnode" functionality so we can actually run this across different networks and find more nodes via the DHT, assuming ports are opened correctly.
  2. An example with two (or maybe three?) binaries, demonstrating dcutr. One binary being the relay and the other two being the two clients wanting to connect to each other. Usecase to be decided, ideally something simple so we don't obscure the details of setting up dcutr.
  3. A client-server example where we demonstrate rendezvous with:
    a. A separate rendezvous node
    b. One binary registering a service
    c. Another binary performing the lookup and connecting to it
  4. One example where we implement a protocol completely from scratch with a NetworkBehaviour, ConnectionHandler, etc

When focusing on the fuller examples, we may not be able to cover all of the current minor examples. Could it be an option to keep some of these around and move them to their respective sub-crates? This goes against your proposal: it would mean an example is either a full skeleton in a separate binary, or a minor component example, part of its own crate. So the central examples still go away.

This is what we have been doing so far but it just confuses users. It also created circular dependencies in our workspace which we just removed. I think it is okay to not cover every protocol in detail. We can compensate this with more docs and perhaps some smaller examples on individual functions but overall, trying to cover every variation in how you can use a protocol is too much repetition of the overall composition code and doesn't help users in deciding on an architecture for how to use rust-libp2p.

@thomaseizinger
Copy link
Contributor

Closed because stale.

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.

4 participants