Continue the RecursiveFindContent query until the uTP transfer is complete and the content is validated #1395
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What was wrong?
Work toward #1383
How was it fixed?
This PR will delay completion of RFC until data is validated, but will not continue the search with other peers. I decided it was getting too big. I will do the follow-up work in another PR.
There are quite a few moving parts here, so the goal was to minimize the
number of functional changes in this refactor.
Overall: add a new query state for while the content is being validated.
Don't exit the query until validation is complete.
Some other changes along the way:
Moving the bigendian conversion of connection ID
Note that when handling the FindContentQueryResponse::ConnectionId in
findcontent.rs, we don't do the bigendian conversion anymore. That has
been moved to earlier in the process, when handling the Content message,
to reduce the number of places that the conversion needs to be handled.
The query trace has slightly different end-stage behavior
The received_from peer and the cancelled peers are not set in the query
until the content gets validated.
Naturally, tests need to reflect the new query structure
For example:
progress
during the test
bugfix: the content search continues even if many peers don't have it
Formerly, the content query would exit out if too many peers replied
successfully (even if the responses were just more nodes to look up).
Best we can tell, the limit on the number of maximum results is
copy-pasta from the recursive-find-nodes logic. (Confirmation from
Mike)
Now, the query continues until the content is found. It doesn't track
the number of successful responses at all.
(Perhaps worth noting that tests fail without this last change. I didn't look that hard into why they weren't failing before)
Refactor
At first, I was trying to change as little as possible. That meant that I kind of shoe-horned the pending result into the Query::Result enum. It works, but is ugly, because of having to handle the pending results in some logic, and the final results in other logic.
So e0e5437 goes a little further and adds a new Pending result enum. It still leaves us with some assertions that certain code paths should be unreachable, but I don't see any better alternatives. I explored another Query trait type, but it required duplicating too much of
query_event_poll
logic. So I landed on this approach, largely implemented by @morph-devTo-Do
if *count >= self.config.num_results {