Send crawled transaction IDs to downloader #2801
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jvff
changed the title
teor2345
reviewed
Sep 28, 2021
3 tasks
jvff
force-pushed
the
send-crawled-transaction-ids-to-downloader
branch
from
9ad2e55 to
9cb7499
Compare
|
I updated the PR to include a test to ensure the crawler survives forwarding errors. This was a bit harder than I expected, and I ended up refactoring the tests a bit to try to make it readable. Hopefully it's not too complicated 😅 You can see only the changes here. |
jvff
commented
Sep 30, 2021
conradoplg
reviewed
Sep 30, 2021
teor2345
reviewed
Oct 1, 2021
|
Thanks again for the comprehensive tests here, I think they'll really help us get the error handling right. |
Replace the single letter with a proper name.
The type names will conflict with the ones for the mempool service.
Add a field to the `Crawler` type to store a way to access the `Mempool` service.
The crawled transactions are now sent to the transaction downloader and verifier, to be included in the mempool.
Make it simpler to use the `MockService::expect_request` method.
Create some dummy crawled transactions, and let the crawler discover them. Then check if they are forwarded to the mempool to be downloaded and verified.
Ignore response from peers that don't provide any crawled transactions.
Calling the Mempool service should not fail, so if an error happens it should be visible. However, errors when downloading individual transactions can happen from time to time, so there's no need for them to be very visible.
Provide some depth as to what the test expect from the crawler's behavior.
Make it easier to reuse the common test setup code.
Now that `zebra_network::Request` implement `Eq`, the call can be simplified into `expect_request`.
A helper function that checks for a network crawl request and responds with the given list of crawled transaction IDs.
A function to intercept and respond to the fanned-out requests sent during a single crawl iteration.
Reduce the repeated code necessary to intercept and reply to a request for queuing transactions to be downloaded.
Intercepts a mempool request to queue transactions to be downloaded, and responds with an error, simulating an internal problem in the mempool service implementation.
This is required for deriving `Arbitrary` for some error types.
Allow random transaction errors to be generated for property tests.
Allow random Mempool errors to be generated for property tests.
The crawler should be robust enough to continue operating even if the mempool service fails to download transactions or even fails to handle requests to enqueue transactions.
They should happen regularly, so there's no need to have them with a high visibility level. Co-authored-by: teor <[email protected]>
If `Mempool::poll_ready` returns an error, it's because the mempool service has stopped and can't handle any requests, so the crawler should stop as well.
jvff
force-pushed
the
send-crawled-transaction-ids-to-downloader
branch
from
f668cde to
ddad6d1
Compare
teor2345
approved these changes
Oct 5, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Motivation
The mempool crawler currently only logs crawled transaction IDs, but doesn't forward them so that the transactions can be downloaded and stored in the mempool.
This closes #2650.
Solution
The
mempool::Crawlerwas updated to receive a handle to the mempool service, and after it receives crawled transactions it just forwards them to be queued in the mempool's transaction downloader.Review
Anyone working on the mempool can review this PR.
Reviewer Checklist
Follow Up Work