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 peer-resolver lib #640

Open
wants to merge 16 commits into
base: develop
Choose a base branch
from

Conversation

aidan46
Copy link
Contributor

@aidan46 aidan46 commented Dec 17, 2024

Description

Add the peer-resolver library with bootstrap and client nodes using the rendezvous and identify protocols for peer discovery. Examples for bootstrap, identification and discovery have been added.

@aidan46 aidan46 added the enhancement New feature or request label Dec 17, 2024
@aidan46 aidan46 added this to the Phase 3 milestone Dec 17, 2024
@aidan46 aidan46 self-assigned this Dec 17, 2024
@aidan46 aidan46 linked an issue Dec 17, 2024 that may be closed by this pull request
@aidan46 aidan46 force-pushed the feat/639-resolver-lib-rendezvous branch from 6ac62f9 to e423d2b Compare December 17, 2024 10:52
Copy link
Collaborator

@jmg-duarte jmg-duarte left a comment

Choose a reason for hiding this comment

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

Preliminary remarks, good job on figuring this out! 🚀

lib/peer-resolver/README.md Show resolved Hide resolved
lib/peer-resolver/src/client.rs Outdated Show resolved Hide resolved
lib/peer-resolver/examples/discovery.rs Show resolved Hide resolved
lib/peer-resolver/examples/discovery.rs Outdated Show resolved Hide resolved
lib/peer-resolver/src/bootstrap.rs Outdated Show resolved Hide resolved
@aidan46 aidan46 marked this pull request as ready for review December 19, 2024 04:50
@aidan46 aidan46 added the ready for review Review is needed label Dec 19, 2024
@aidan46 aidan46 force-pushed the feat/639-resolver-lib-rendezvous branch from 0922b1b to fd7f21f Compare December 20, 2024 04:44
@aidan46 aidan46 added ready for review Review is needed and removed ready for review Review is needed labels Dec 20, 2024
@aidan46 aidan46 added ready for review Review is needed and removed ready for review Review is needed labels Dec 20, 2024
@aidan46 aidan46 added ready for review Review is needed and removed ready for review Review is needed labels Dec 20, 2024
@aidan46 aidan46 added ready for review Review is needed and removed ready for review Review is needed labels Dec 20, 2024
Copy link
Collaborator

@jmg-duarte jmg-duarte left a comment

Choose a reason for hiding this comment

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

One question I have is — what is a cookie in this context?

lib/peer-resolver/src/discovery.rs Outdated Show resolved Hide resolved
}

/// This functions requests the rendezvous_point for new or updated peers since the last cookie.
pub fn discover(&mut self, rendezvous_point: PeerId) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm not sure this function name is good, it isn't really descriptive of what it does.
I know it wraps discover but I'm not convinced by libp2p's naming.

lib/peer-resolver/src/register.rs Show resolved Hide resolved
lib/peer-resolver/src/register.rs Show resolved Hide resolved
lib/peer-resolver/src/register.rs Show resolved Hide resolved
Comment on lines +21 to +23
swarm
.register(rendezvous_point, rendezvous_point_address, Some(MAX_TTL))
.await?;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Does this simply "stay on"? In other words, when does this function call exit?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This function call exits as soon as the peer is registered. The peer knows this when it receives the Registered event from the rendezvous point.

@aidan46
Copy link
Contributor Author

aidan46 commented Dec 20, 2024

One question I have is — what is a cookie in this context?

A cookie is passed into the discover function to only receive peers that have changed or joined since the last discover message was served.

Copy link
Member

@cernicc cernicc left a comment

Choose a reason for hiding this comment

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

Try to create a single example of using all these behaviors and you'll see how do you like the usage of the current abstraction. I have a feeling it would be easier to have a single behavior that specifies how the node operates.

@@ -0,0 +1,75 @@
# Peer Resolver

This library is intended for peer discovery in a libp2p network.
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you remind me what is the expected usage of this library in our storage solution?
It's kinda hard to imagine for me whether those abstractions are correct, if we write the library first and the usage second.
This looks like a nice example, but not sure whether it will fit/how it will fit in our whole system.

lib/peer-resolver/src/bootstrap.rs Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request ready for review Review is needed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

feat: libp2p rendezvous discovery
4 participants