Add Kademlia mode override. #1736
Conversation
| @@ -224,6 +225,8 @@ pub struct Config<ProviderStorage> { | |||
| pub special_target_connections: u32, | |||
| /// Addresses to bootstrap Kademlia network | |||
| pub bootstrap_addresses: Vec<Multiaddr>, | |||
| /// Optionally overrides the default Kademlia server mode. | |||
| pub kademlia_mode_override: Option<Option<Mode>>, | |||
There was a problem hiding this comment.
Why not just Option<Mode> and default it to Some(Mode::Client)?
There was a problem hiding this comment.
None is a valid Kademlia mode (automatic mode). This way I can set three modes as well as use the default.
There was a problem hiding this comment.
Why do we need automatic mode? I thought the whole point was to make it explicit
There was a problem hiding this comment.
Initially, I set the Server mode to upgrade the libp2p with preserving the current behavior. However, It would be great to use the automatic mode, otherwise, we should add basically the same code as they did. On the other hand, when we know that we must not get to the routing table (as for temporary clients now or for nodes in the future) it's convenient to set Client mode explicitly. I believe that having all options available is good.
There was a problem hiding this comment.
It still looks very strange. If we remove outer Option we just don't set anything on None (libp2p default) and if it is Some then we do set it to whatever was specified. I don't think Some(None) is useful.
There was a problem hiding this comment.
I change the semantics then.
| @@ -340,7 +340,7 @@ where | |||
| general_target_connections: SWARM_TARGET_CONNECTION_NUMBER, | |||
| special_target_connections: SWARM_TARGET_CONNECTION_NUMBER, | |||
| bootstrap_addresses: Vec::new(), | |||
| kademlia_mode_override: None, | |||
| kademlia_mode: Some(Mode::Server), | |||
There was a problem hiding this comment.
Hm... I recall we wanted the default to be client, not server. Why this change? If we want to keep libp2p's default then we need to leave None.
This PR adds an optional Kademlia mode override. The feature allows using temporary custom clients without polluting the Kademlia DHT routing table.
Libp2p v0.52.1 is able to determine the Kademlia mode automatically which would be a preferred solution. However, we need to switch to the automatic adding of the addresses by Kademlia and support Autonat to support the automatic mode switch.
Code contributor checklist: