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

kad: remove default IPFS DHT protocol name #5006

Closed
mxinden opened this issue Dec 14, 2023 · 3 comments · Fixed by #5122
Closed

kad: remove default IPFS DHT protocol name #5006

mxinden opened this issue Dec 14, 2023 · 3 comments · Fixed by #5122
Labels
difficulty:easy getting-started Issues that can be tackled if you don't know the internals of libp2p very well help wanted
Milestone

Comments

@mxinden
Copy link
Member

mxinden commented Dec 14, 2023

Today a new kad::Config will use the protocol name of the IPFS DHT, namely /ipfs/kad/1.0.0, by default.

This is problematic in the case where one wants to connect to a different DHT instance, or launch their own. E.g. in the past, new DHT networks have accidentally merged with the IPFS DHT instance.

To prevent this from happening in the future, we should not provide a default protocol name, but instead require a user to explicitly specify the protocol name and thus the DHT instance that they would like to join.

One possible fix would be to introduce libp2p_kad::Config::new taking a protocol name and to remove the Default implementation on libp2p_kad::Config.

/// The configuration for the `Kademlia` behaviour.
///
/// The configuration is consumed by [`Behaviour::new`].
#[derive(Debug, Clone)]
pub struct Config {
kbucket_pending_timeout: Duration,
query_config: QueryConfig,
protocol_config: ProtocolConfig,
record_ttl: Option<Duration>,
record_replication_interval: Option<Duration>,
record_publication_interval: Option<Duration>,
record_filtering: StoreInserts,
provider_record_ttl: Option<Duration>,
provider_publication_interval: Option<Duration>,
kbucket_inserts: BucketInserts,
caching: Caching,
}
impl Default for Config {
fn default() -> Self {
Config {
kbucket_pending_timeout: Duration::from_secs(60),
query_config: QueryConfig::default(),
protocol_config: Default::default(),
record_ttl: Some(Duration::from_secs(36 * 60 * 60)),
record_replication_interval: Some(Duration::from_secs(60 * 60)),
record_publication_interval: Some(Duration::from_secs(24 * 60 * 60)),
record_filtering: StoreInserts::Unfiltered,
provider_publication_interval: Some(Duration::from_secs(12 * 60 * 60)),
provider_record_ttl: Some(Duration::from_secs(24 * 60 * 60)),
kbucket_inserts: BucketInserts::OnConnected,
caching: Caching::Enabled { max_peers: 1 },
}
}
}

@mxinden mxinden added difficulty:easy help wanted getting-started Issues that can be tackled if you don't know the internals of libp2p very well labels Dec 14, 2023
@mxinden mxinden added this to the v0.54.0 milestone Dec 14, 2023
@robin-rrt
Copy link

This looks simple enough. I'd like to work on this. Can I be assigned?

@dariusc93
Copy link
Contributor

Sounds like it can mimic what libp2p-identify does by defining a protocol name, though in this case it would be an array (unless this will be cut back to just be a single define protocol for libp2p-kad). Honestly, it sounds like a better idea than relying on the default.

@thomaseizinger
Copy link
Contributor

though in this case it would be an array (unless this will be cut back to just be a single define protocol for libp2p-kad)

We added this back in #2837. I am wondering if we couldn't achieve the same thing by having multiple instances of libp2p_kad::Behaviour in your behaviour tree, all with a differently configured protocol name.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
difficulty:easy getting-started Issues that can be tackled if you don't know the internals of libp2p very well help wanted
Projects
None yet
4 participants