Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

kad: Query::next_action() message is ignored by the kademlia handler #230

Open
Tracked by #307
lexnv opened this issue Sep 3, 2024 · 0 comments
Open
Tracked by #307

kad: Query::next_action() message is ignored by the kademlia handler #230

lexnv opened this issue Sep 3, 2024 · 0 comments
Labels
enhancement New feature or request

Comments

@lexnv
Copy link
Collaborator

lexnv commented Sep 3, 2024

Query::next_action() method returns a QueryAction that contains a raw kademlia message.
For the FindNode query type it is the FIND_NODE message, for GetRecords query type it is the GET_VALUE message.

pub fn next_action(&mut self) -> Option<QueryAction> {

The message is ignored by the kademlia handler on_query_action method:

while let Some(action) = self.engine.next_action() {
if let Err((query, peer)) = self.on_query_action(action).await {
self.disconnect_peer(peer, Some(query)).await;
}
}

The following lines are ignoring the provided message, and sending the FindNode request regardless of the message provided:

QueryAction::SendMessage { query, peer, .. } => {
if self
.open_substream_or_dial(peer, PeerAction::SendFindNode(query), Some(query))
.is_err()
{
// Announce the error to the query engine.
self.engine.register_response_failure(query, peer);
}

The intention behind this behavior was to advance the discoverability of the network. For example, GetRecords query must first discover the network, then publish a GET_VALUE request.

Consider untangling the query message with the kademlia handler and to allow queries to decide which message should be sent.
We can further look into optimizing the number of messages sent from the query engine to the kademlia handler (since some of them may be redundant as in this case)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

1 participant