Skip to content
This repository has been archived by the owner on Nov 6, 2020. It is now read-only.

Distance between nodes in node discovery #8721

Closed
ayrat555 opened this issue May 28, 2018 · 2 comments
Closed

Distance between nodes in node discovery #8721

ayrat555 opened this issue May 28, 2018 · 2 comments
Labels
F3-annoyance 💩 The client behaves within expectations, however this “expected behaviour” itself is at issue. M4-core ⛓ Core client code / Rust. P5-sometimesoon 🌲 Issue is worth doing soon.
Milestone

Comments

@ayrat555
Copy link
Contributor

https://github.com/paritytech/parity/blob/master/util/network-devp2p/src/discovery.rs#L236

It looks like the distance between nodes is calculated as a common prefix of each byte. Shouldn't it be just a common binary prefix?

I wrote a test

    #[test]
    fn test_distance() {
        let id1 = H256::from(1_000_000_000);
        let id2 = H256::from(9_000_000_000);

        let distance = Discovery::distance(&id1, &id2);

        assert!(distance == 2);
    }

1_000_000_000 - [0, 0, 1, 1, 1, 0, 1, 1, 1, 0, 0, 1, 1, 0, 1, 0, 1, 1, 0, 0, 1, 0, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0]
9_000_000_000 - [0, 0, 0, 0, 0, 0, 1, 0, 0, 0, 0, 1, 1, 0, 0, 0, 0, 1, 1, 1, 0, 0, 0, 1, 0, 0, 0, 1, 1, 0, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0]

So the common prefix is 2.
Discovery::distance returns 24

@Tbaut Tbaut added Z1-question 🙋‍♀️ Issue is a question. Closer should answer. M4-core ⛓ Core client code / Rust. labels May 28, 2018
@Tbaut Tbaut added this to the 1.12 milestone May 28, 2018
@sorpaas
Copy link
Collaborator

sorpaas commented May 29, 2018

Might be related to #8589

@Tbaut Tbaut added P5-sometimesoon 🌲 Issue is worth doing soon. F3-annoyance 💩 The client behaves within expectations, however this “expected behaviour” itself is at issue. and removed Z1-question 🙋‍♀️ Issue is a question. Closer should answer. labels May 29, 2018
@5chdn 5chdn modified the milestones: 2.0, 2.1 Jul 17, 2018
@5chdn 5chdn modified the milestones: 2.1, 2.2 Sep 11, 2018
@5chdn 5chdn modified the milestones: 2.2, 2.3 Oct 29, 2018
@sorpaas
Copy link
Collaborator

sorpaas commented Nov 15, 2018

From the literature I can find, Kademlia distance are defined via XOR metric. In Parity Ethereum we used log2(XOR) which should have similar effect. So looks like this function is correct.

Please feel free to re-open this issue if you find out I'm wrong!

@sorpaas sorpaas closed this as completed Nov 15, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
F3-annoyance 💩 The client behaves within expectations, however this “expected behaviour” itself is at issue. M4-core ⛓ Core client code / Rust. P5-sometimesoon 🌲 Issue is worth doing soon.
Projects
None yet
Development

No branches or pull requests

4 participants