Add convenience method to generate a SHA256 Key256 from a byte slice

[dennis-tra]

No description provided.

[guillaumemichel]

IMO this function should be located in the DHT implementation. The identity generation isn't the responsibility of go-kademlia.

Moreover, we should try to keep generic features here. We don't want to add identity generation for multiple hash functions.

[dennis-tra]

IMO this function should be located in the DHT implementation.

Since it's only a convenience function, I can't argue much against this. I just think SHA256 is so wide-spread that it warrants special treatment.

The identity generation isn't the responsibility of go-kademlia.

This is just the SHA256 of some byte slice. The interpretation that this is the identity of something still resides in go-libp2p-kad-dht.

Moreover, we should try to keep generic features here. We don't want to add identity generation for multiple hash functions.

Totally agree with the first part. Weakly opposing the second part. If you feel strongly about this, I don't mind keeping it out of go-kademlia 👍

[guillaumemichel]

It is also common to hash bytes along with some salt. I don't think it is good to add these functions to go-kademlia, because if users start depending on it we cannot modify nor deprecate them, and we will need to add new standard with time.

I guess that it would be more convenient to have helper functions PeeridToKadKey and CidToKadKey directly on go-libp2p-kad-dht. This way, it is easier to update the identifier of different elements individually. E.g with double hashing, the peer kademlia identifiers will remain the same, but the content kademlia identifier will change (the hash will be salted).

Otherwise, using wrappers also works to provide a mapping to kademlia keys (see example), and it is also convenient to modify.

[dennis-tra]

It is also common to hash bytes along with some salt.

Here, the salt would be part of the data you pass into NewSha256

I don't think it is good to add these functions to go-kademlia, because if users start depending on it we cannot modify nor deprecate them, and we will need to add new standard with time.

Keeping a small API surface is certainly good to gravitate to 👍 though the idea of adding this function is so that users depend on it.

I guess that it would be more convenient to have helper functions PeeridToKadKey and CidToKadKey directly on go-libp2p-kad-dht. This way, it is easier to update the identifier of different elements individually. E.g with double hashing, the peer kademlia identifiers will remain the same, but the content kademlia identifier will change (the hash will be salted).

I'm doing what you have linked in the example. This is what I have in go-libp2p-kad-dht right now:

// nodeID is a type alias for peer.ID that implements the kad.NodeID interface.
// This means we can use nodeID for any operation that interfaces with
// go-kademlia.
type nodeID peer.ID

// assertion that nodeID implements the kad.NodeID interface
var _ kad.NodeID[key.Key256] = nodeID("")

// Key returns the Kademlia key of nodeID. The IPFS (amino?) DHT operates on SHA256
// hashes of, in this case, peer.IDs. This means this Key method takes
// the peer.ID, hashes it and constructs a 256-bit key.
func (p nodeID) Key() key.Key256 {
	return key.NewSha256([]byte(p)) // though, this is the function in question - I'm using this in two other places as well.
}

// String calls String on the underlying peer.ID and returns a string like
// QmFoo or 12D3KooBar.
func (p nodeID) String() string {
	return peer.ID(p).String()
}

My plan was to use key.NewSha256 in handleGetValue and handleGetProviders where the key that we receive in the request is just an arbitrary byte slice. To derive the kademlia key I would have used NewSha256. However, I sense that you don't think it's a good idea to have it in go-kademlia. I'll just move the function to go-libp2p-kad-dht. I'd reopen this if I need it in more places.