feat: logic for atoma p2p for gossip nodes public urls

[jorgeantonio21]

No description provided.

[jorgeantonio21]

@sdbondi I added here error handling for message validation, and providing the acceptance criteria resulting from it, which is then passed below in the report_message_validation_result. How does this look like ?

[jorgeantonio21]

@sdbondi also added a report_message_validation_result to always accept requests for node addresses, as if I understand correctly, otherwise this message would not be propagated to other nodes.

[sdbondi]

That is correct - however since gossipsub gossips the message to all subscribers, you probably don't want to use it for this part of the protocol. i.e. a single request will arrive at all subscribers (i.e all nodes) and will result in all nodes sending a response to all nodes. I assume that all nodes also send a request. $O(n^2)$ message complexity.

https://docs.rs/libp2p-request-response/latest/libp2p_request_response/ might be what you're looking for

[jorgeantonio21]

@sdbondi publishing new messages does not require further validation, am I correct ?

[maschad]

I'm still not convinced having p2p nodes register their public url is an effective mechanism for peer routing

If we are going to be using Kademlia DHTs for discovery this seems counter intuitive, nodes should be located by their PeerID and the content of that peer's route be stored upon discovery.

We can use the NewListenAddress event to dial them upon discovery.

It may be useful to examine how Kademlia works so that this is clearer

EDIT: After discussing with @jorgeantonio21 we decided that for this first iteration we would like to leverage the easy API request routing via axum and then we can move to a more decoupled seperation of concerns with the proxy having an exposed compatible [OpenAPI}(https://www.openapis.org/) which will then routing requests to the nodes, which will be discovered via Kademlia and connected in mesh.

[maschad]

Awesome work on this @jorgeantonio21 🚀

Looking good but left some comments

[maschad]

I think there are quite a few more events we will want to explicitly handle here, namely:

     SwarmEvent::Behaviour(MyBehaviourEvent::Mdns(mdns::Event::Discovered(list))) => {
                    for (peer_id, _multiaddr) in list {
                       debug!(
                                target = "atoma-p2p",
                                event = "mdns_discovered",
                                peer_id = %peer_id,
                                "mDNS discovered a new peer"
                            );
    			// Let's explicitly add peers discovered that, they may allow us to find new peers or they may be already subscribed to the topic
                        swarm.behaviour_mut().gossipsub.add_explicit_peer(&peer_id);
                    }
                },
                SwarmEvent::Behaviour(MyBehaviourEvent::Mdns(mdns::Event::Expired(list))) => {
                    for (peer_id, _multiaddr) in list {
                      debug!(
                                target = "atoma-p2p",
                                event = "mdns_expired",
                                peer_id = %peer_id,
                                "mDNS  peer expired"
                            );
                        swarm.behaviour_mut().gossipsub.remove_explicit_peer(&peer_id);
                    }
                },
                  SwarmEvent::NewListenAddr { address, .. } => {
                      debug!(
                                target = "atoma-p2p",
                                event = "new_listen_address",
                                peer_id = %peer_id,
                                ""
                            );
                }
                SwarmEvent::Dialing {
                peer_id: Some(peer_id),
                ..
            } => debug!("Dialing {peer_id}"),
            e => panic!("{e:?}"),

[jorgeantonio21]

Are all these the events to handle ? If I understand correctly gossipsub already integrates kademlia for peer discovery, so there seems to not be much to do on that front..

[maschad]

That's inaccurate for rust-libp2p see libp2p/rust-libp2p#5313

At the moment one has to explicitly add them when the Discovered event is emitted as you can see the discovered_nodes is not merged into the gossipsub nor Kademlia properties, which is why in this example it's done explicitly

EDIT: mdns will update the gossipsub mesh but not Kademlia, so this still applies here for that use case.

[jorgeantonio21]

I believe we don't need to update kademlia for any new mdns discovery, as these peers are mostly residing on a local network. What do you think ?

[maschad]

I disagree I think it will help because the bootstrap nodes will now have any locally discovered nodes, which will help any future node during it's bootstrap node.

[maschad]

LGTM minus this one comment #286 (comment) Great job @jorgeantonio21 🚀

[maschad]

Excellent work!