Skip to content

Commit

Permalink
kad: Refactor FindNode query, keep K best results and add tests (#114)
Browse files Browse the repository at this point in the history
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

```rust
// 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

---------

Signed-off-by: Alexandru Vasile <[email protected]>
  • Loading branch information
lexnv authored May 24, 2024
1 parent 92cba54 commit e022410
Show file tree
Hide file tree
Showing 3 changed files with 453 additions and 112 deletions.
Loading

0 comments on commit e022410

Please sign in to comment.