libp2p/specs

Allow peer to find itself in FIND_NODE

sgdxbc opened this issue · 3 comments

sgdxbc commented

In libp2p/rust-libp2p#3692 I was informed that several implementation exclude the calling peer from FIND_NODE result, and this behavior is not required by spec and does not make sense to me.

In my use case, I want to get consistent FIND_NODE result no mater which peer is calling it. However when local peer is indeed the closest peer, e.g., the input hash is identical to local peer's id, the result from local peer is different from the results from others.

Instead of requiring a change in a libp2p implementation directly, I suggest wrapping the Kademlia::find_node method where that wrapper adds the local peer ID in case it was requested.

sgdxbc commented

That's exactly how I workaround for my use case currently, my code for reference

match event {
  SwarmEvent::Behaviour(AppEvent::Kad(KademliaEvent::OutboundQueryProgressed {
      id,
      result: QueryResult::GetClosestPeers(result),
      ..
  })) if *id == find_id => {
      let Ok(result) = result else {
          unimplemented!()
      };
      // kad excludes local peer id from `GetClosestPeers` result for unknown reason
      // so by default, the result from closest peers themselves is different from the others
      // add this workaround to restore a consistent result
      let mut peers = result.peers.clone();
      let k = |peer_id: &PeerId| {
          kbucket::Key::from(*peer_id).distance(&kbucket::Key::from(key))
      };
      let local_id = *swarm.local_peer_id();
      let index = peers.binary_search_by_key(&k(&local_id), k).expect_err(
          "local peer id is always excluded from get closest peers result",
      );
      peers.insert(index, local_id);
      peers.pop();
      // ...
  }
  // ...
})
sgdxbc commented

I am not asking for implementations to change their behavior as long as their current behavior fits the common demand for this interface (which is probably true but I'm not sure). The key is to keep consistency between spec and implementations so other people will not get surprised by the result after reading the spec like I did.

So I can create a PR to mention this behavior in the spec if desired (not sure about the exact workflow to update spec but always here to help). And I am also curious about why implementation choose to do the exclusion explicitly. rust-libp2p says it following go-libp2p, and that's all I know.