search

rust-p2p / s-kademlia

Implementation of the s/kademlia protocol
9 stars 2 forks source link

Content Addressed Hash (MultiHash) vs DiscoHash #6

Closed 4meta5 closed 5 years ago

4meta5 commented 5 years ago

libp2p-kad has a PeerId like

#[derive(Clone, PartialEq, Eq, Hash)]
pub struct PeerId {
    multihash: multihash::Multihash,
}

I was and still am considering using a similar structure for NodeId with the field id: disco::DiscoHash, but I actually think it would be better to make this generic over some trait that both multihash::MultiHash and disco::DiscoHash conform to.

partition network by data flow types

Some data would benefit from MultiHash while other data benefits from disco::DiscoHash. I suspect that some parts of the DHT would benefit from self-describing hashes. Maybe they can be layered based on id type. What do you think @dvc94ch ?

dvc94ch commented 5 years ago

The problem with multihash, is that there are three versions floating around already. The official one, the rust-libp2p one and the rust-ipfs one. At this point you may consider it a failure. If you want to add discohash to multihash that's another version. Besides we don't plan on supporting every possible hash, but it may be useful to support a version tag, such that v0 => discohashv0, v1 => discohashv1

4meta5 commented 5 years ago

I don't see that as a huge problem and am in favor of adding discohash to multihash. I don't think we need to specify a version tag if the hash algorithm is a field in the struct (as in multihash). I still need to get a better understanding of how the eccentricities of discohash fit in with the data storage/transport requirements of the DHT.

dvc94ch commented 5 years ago

The advantage of the version tag is you always know which hash to use, the latest version. I don't see how supporting all hashes ever invented helps anyone, especially if by support you mean recognize. So to effectively use multihash you need to impl every hash because someone may decide to use it. It also does not make it clear if a hash is broken. People can easily see they are supposed to use the largest version. I'm not sure how adopting this brainfart from ipfs is an advantage

dvc94ch commented 5 years ago

And even support as in recognize, besides it being useless, doesn't work in practice, since everyone just forks multihash and rust-multihash is unmaintained