Kad Import

[guillaumemichel]

See probe-lab/go-kademlia#120 (comment) for reference.

Importing basic Kademlia types, as well as the trie, key and triert packages.

[dennis-tra]

Awesome, thanks @guillaumemichel! This is how I thought the first step could look like. Love the go.mod with just a single test dependency. Let's see how long it'll stay that lean.

Some comments:

• I'd prefer a shallow package hierarchy where the different key types would just live in a single directory, i.e., lift everything from bistrkey, key256, and key/test into the key package/folder. BitStrKey would then become only BitStr or BitString. Key256 would probably stay Key256 because the name cannot start with a number.
• do we want to lift kadtest out of the internal package? We're using things from there in go-libp2p-kad-dht - for example the CtxShort. Probably more @iand might have a better overview?
• Would be great to pull in UCI for automated tests (can be a separate PR)
• There are a couple of interfaces in kad that we don't use anymore afaict. For example, Address and both <X>Protocol interfaces.

[dennis-tra]

@iand I guess we went away from this pattern?

[guillaumemichel]

I'd prefer a shallow package hierarchy where the different key types would just live in a single directory, i.e., lift everything from bistrkey, key256, and key/test into the key package/folder. BitStrKey would then become only BitStr or BitString. Key256 would probably stay Key256 because the name cannot start with a number.

What are the benefits of a shallow package hierarchy?

The benefit I see for the current hierarchy are that users can choose to import only one type of Key and they don't end up importing a large package with many Keys they don't use. I am not against having all implementations in a key package, but would like to understand the benefits.

Would be great to pull in UCI for automated tests (can be a separate PR)

What is the process to pull these?

There are a couple of interfaces in kad that we don't use anymore afaict. For example, Address and both Protocol interfaces.

In this case I suggest to remove the following types:

• NodeInfo
• Address
• Message
• Request
• Response
• RoutingProtocol
• RecordProtocol
• Record

We can implement the necessary types in go-libp2p-kad-dht/v2 and move back some types here once we are happy with them if required.

[iand]

My preference is also for shallower package hierarchies. But I think having a package for distinct types of key makes sense. I see an analogy with the crypto or hash package hierarches in the Go standard lib

However the current types names are awkward to work with and leads to stuttering:

• bitstrkey.BitStrKey
• key256.Key256

I would prefer:

• bitstr.Key

key256 is hard to rename but perhaps this would be nicer

• bit256.Key

[iand]

Approved but I would like to see the naming of the key packages and types improved as per my comment

[guillaumemichel]

Recent changes:

• kadtest package lifted out of internal
• moved bitstrkey.BitStrKey to bitstr.Key
• moved key256.Key256 to bit256.Key
• removed the following unused types:
  • NodeInfo
  • Address
  • Message
  • Request
  • Response
  • RoutingProtocol
  • RecordProtocol
  • Record

WDYT @dennis-tra

[dennis-tra]

Hmm I'd still prefer a single key package :D the crypto packages are more sophisticated than what we're doing with our keys but it seems I'm outvoted :)

Fine with me to merge 👍