-
Notifications
You must be signed in to change notification settings - Fork 107
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
Add a debug config that enables the mempool #2862
Conversation
f6b6f99
to
ce51b29
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I rebased this after #2861 was merged.
This seems good, the only thing lacking is already mentioned as TODO (moving flag into SyncStatus to avoid repetition, and tests).
Something that I realized now and may be useful here (or not, feel free to ignore): if we move the crawler inside Mempool
, then it won't need to monitor the tip anymore, as it will be enabled/disabled with the mempool (though we'd need logic to re-spawn / kill the task when that happens)
(I've just realized that I probably shouldn't have rebased & force-pushed something that it's still a draft, apologies if this makes things a bit difficult on your end @teor2345 😬 ) |
I don't mind, but I'm glad you mentioned it, so I wasn't surprised when I went to push. I can just do |
I really like this idea, and I think it could work. And if we do it, we don't need to modify |
We don't want to ignore these errors, because they might indicate a shutdown. (Or a bug that we should fix.)
ce51b29
to
abb83fe
Compare
This is now ready for review - I want to do the cleanup in a follow-up PR. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good
Motivation
We want to run acceptance tests on the mempool.
To do that, we need to be able to configure the mempool to start quickly.
(Rather than at the chain tip, which takes hours to sync.)
Closes #2690.
Solution
debug_enable_at_height
configdebug_enable_at_height
in the mempool servicedebug_enable_at_height
in the mempool crawlerTests:
debug_enable_at_height
Bug fixes:
I'd like to leave the crawler refactor for my next PR, so we can bisect if it causes issues.
Review
@conradoplg can finish off the review for this PR.
This PR is based on PR #2861.Reviewer Checklist
Follow Up Work
zebrad
acceptance tests for the mempool #2691Refactor the
debug_enable_at_height
checks - choose one of:- move the
debug_enable_at_height
code intoSyncStatus
, so we don't have to copy it everywhere we useSyncStatus
- launch the mempool crawler task when the
Mempool
is enabled, and kill it when it is disabled