kad: Revisit FIND_NODE and increase test coverage
#100
Assignees
Labels
enhancement
New feature or request
Comments
lexnv
added a commit
that referenced
this issue
May 24, 2024
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]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Refactor the
FIND_NODEquery to make the code more robustnext_actionmethod to handle cases in a graceful waySimilar to #97
The text was updated successfully, but these errors were encountered: