kad: Refactor FindNode query, keep K best results and add tests

[lexnv]

This PR refactors the FindNode query to make it more robust.

• Avoid panics on unwraps and unimplemented logic
• Separate config immutable variables from main query logic
• Simplifies logic of next_action method
• Private methods for internal logic

It also fixes a bug where only the last peer ID of our results was updated.

• we were registering the first 20 (repl factor) responses
• whenever we got a new response (better) we tried to update just the peerID_ of the last entry

This had a few implications:

• the first 19 entries are never updated with a better (closer) peer
• the distance of the last entry was never updated. And because of this, we might end up in a situation where the record is updated with a furthest distance

// Presume: 
last_entry: distance = 10, peer = A

// Registering a new response
new_response: distance = 2, peer = B
-> last entry: distance = 10, peer = B

// Registering a new response
new_response: distance = 7, peer = C
-> last entry: distance = 10, peer = C

// we should've kept B as best

Testing Done

• Completes if there's no further candidate to query
• Fulfill number of parallel queries
• Completes on the number of responses
• Provides the closest response out of the batch
• Provides the closest response when the closest peer is indirectly found and K factor is already fulfilled

Closes #100

cc @paritytech/networking

[dmitry-markin]

Nice PR! And good to see we have FIND_NODE tested now.

[dmitry-markin]

Is this TODO still relevant? It looks like nothing has changed in its respect.

[lexnv]

I dont think this is addressed indeed, will briefly look over libp2p or create an issue if this may be too involved / not covered by our testing atm, thanks!

[lexnv]

While looking at this I realized a few things:

• we were registering the first 20 (repl factor) responses
• whenever we got a new response (better) we tried to update just the peerID_ of the last entry

This has a few implications:

• the first 19 entries are never updated with a better (closer) peer
• the distance of the last entry was never updated. And because of this, we might end up in a situation where the record is updated with a furthest distance

// Presume: 
last_entry: distance = 10, peer = A

// Registering a new response
new_response: distance = 2, peer = B
-> last entry: distance = 10, peer = B

// Registering a new response
new_response: distance = 7, peer = C
-> last entry: distance = 10, peer = C

// we should've kept B as best

In this case, we should have updated other records and only keep the window of the best 20 peers.

To mitigate this, if we receive a response better than the last entry:

• insert the response with distance and peer to our btreemap, if and only if we didn't track that distance before
• pop the last entry to keep only K entries

Have also added a test for this keep_k_best_results

[dmitry-markin]

What is the drawback of overwriting the entry with this distance? I.e., there should be exactly one peer with such distance as it uniquely identifies the hash, so it should be safe to overwrite without checking.

[lexnv]

I think I followed the libp2p here. However, I believe we can overwrite the entries without checking here, and we'll have a "fresher" report in the end. Will do that, thanks!