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

Create initial transaction crawler for the mempool #2646

Merged
merged 7 commits into from
Aug 24, 2021

Conversation

jvff
Copy link
Contributor

@jvff jvff commented Aug 20, 2021

Motivation

As part of the work to implement a mempool for Zebra, there's a need to crawl peers for transactions to populate the mempool. The mempool crawler is a background task that periodically asks a few peers for transactions.

Specifications

The Bitcoin documentation contains a small description about the mempool related network messages.

Designs

Solution

A Crawler type is created inside the mempool component. Calling Crawler::spawn will construct the type and spawn a task to run it on the background. The crawler periodically requests a few peers (configured by a fan-out constant) for transaction IDs.

There's a single test to make sure that the requests are sent in a timely manner to the PeerSet service.

Review

@teor2345 has been following the design of this, but anyone else can chime in as well.

Reviewer Checklist

  • Code implements Specs and Designs
  • Tests for Expected Behaviour
  • Tests for Errors

Follow Up Work

@jvff jvff requested a review from teor2345 August 20, 2021 02:07
@zfnd-bot zfnd-bot bot assigned jvff Aug 20, 2021
Copy link
Contributor

@teor2345 teor2345 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This all looks amazing!

I have a few suggestions about timeouts and error handling.

zebrad/Cargo.toml Outdated Show resolved Hide resolved
zebrad/src/components/mempool/crawler.rs Outdated Show resolved Hide resolved
zebrad/src/components/mempool/crawler.rs Outdated Show resolved Hide resolved
zebrad/src/components/mempool/crawler.rs Outdated Show resolved Hide resolved
zebrad/src/components/mempool/crawler.rs Show resolved Hide resolved
zebrad/src/components/mempool/crawler.rs Outdated Show resolved Hide resolved
@jvff jvff force-pushed the mempool-crawler branch from 3f1b488 to 25be56b Compare August 20, 2021 23:02
@jvff
Copy link
Contributor Author

jvff commented Aug 20, 2021

I've updated the code. I don't know if this is useful, but here's a link to what has changed: https://github.com/jvff/zebra/compare/mempool-crawler-r1..mempool-crawler-r1-fixes.

@jvff jvff marked this pull request as ready for review August 20, 2021 23:18
Copy link
Contributor

@teor2345 teor2345 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've suggested some more changes, but happy with whatever you want here.

zebrad/src/components/mempool/crawler.rs Show resolved Hide resolved
zebrad/src/components/mempool/crawler.rs Show resolved Hide resolved
zebrad/src/components/mempool/crawler.rs Outdated Show resolved Hide resolved
@mpguerra mpguerra linked an issue Aug 23, 2021 that may be closed by this pull request
jvff and others added 7 commits August 23, 2021 17:40
The mempool crawler is responsible for periodically asking peers for
transactions to insert into the local mempool. This initial
implementation will periodically ask for transactions, but won't do
anything with them yet.

Also, the crawler is currently configured to be always enabled, but this
should be fixed to avoid crawling while Zebra is still syncing the
chain.
Prevent the crawler from getting stuck if there's communication with a
peer that takes too long to respond.
Spawn a task for the crawler when Zebra starts.
Create a mock for the `PeerSet` service to intercept requests and verify
that the transaction requests are sent periodically.
Make it simpler to select the features for test builds.

Co-authored-by: teor <[email protected]>
Make it easy to navigate from the `TODO` comment to the current project
planning.

Co-authored-by: teor <[email protected]>
Make it easy to navigate from the `TODO` comment to the current project
planning.

Co-authored-by: teor <[email protected]>
@jvff jvff force-pushed the mempool-crawler branch from 25be56b to 8a01c95 Compare August 23, 2021 17:41
@jvff
Copy link
Contributor Author

jvff commented Aug 23, 2021

Updated.

@jvff jvff requested a review from teor2345 August 23, 2021 22:01
Copy link
Contributor

@teor2345 teor2345 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm fine with whatever workaround you want to use until tokio is updated.

Feel free to merge with the Mutex workaround, or to use the &mut self workaround in PR #2657.

@jvff jvff merged commit 069f771 into ZcashFoundation:main Aug 24, 2021
@jvff jvff deleted the mempool-crawler branch August 24, 2021 14:23
Copy link
Contributor

@dconnolly dconnolly left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

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

Successfully merging this pull request may close these issues.

Implement Mempool Crawler Task
3 participants