-
Notifications
You must be signed in to change notification settings - Fork 117
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
Clear the mempool by using a sync method #2777
Conversation
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.
This looks good, there are just a few tricky edge cases we need to handle.
Co-authored-by: teor <teor@riseup.net>
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.
Seems like I gave some bad advice here - we can't use unconstrained
until we upgrade tokio.
But the alternative refactor gives us simpler and faster code, so let's just do that.
All suggestions should be implemented now. |
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.
LGTM. I added a few optional suggestions, but they're minor.
Co-authored-by: Janito Vaqueiro Ferreira Filho <janito.vff@gmail.com>
Now I'm not sure whether to merge this into |
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.
We could merge this as-is, but I'd like to fix a potential future performance issue.
(I expect we'll be using last_tip_change
a lot, at the same time as the state is committing thousands of blocks.)
We could:
But I'm happy to let @conradoplg make the final call here, because #2764 is his PR. |
I agree with the plan 👍 I can merge this after that last required change |
Co-authored-by: teor <teor@riseup.net>
@conradoplg both PRs should be ready. |
* Update the expiry TODO * Clear the mempool at a chain tip reset * Clear the mempool by using a sync method (#2777) * Clear the mempool by using a sync method * Update docs * Apply suggestions from code review Co-authored-by: teor <teor@riseup.net> * Refactor last_tip_change() * Apply suggestions from code review Co-authored-by: Janito Vaqueiro Ferreira Filho <janito.vff@gmail.com> * Fix brackets * Use best_tip_block instead of manual borrowing Co-authored-by: teor <teor@riseup.net> Co-authored-by: teor <teor@riseup.net> Co-authored-by: Janito Vaqueiro Ferreira Filho <janito.vff@gmail.com> Co-authored-by: teor <teor@riseup.net> Co-authored-by: Janito Vaqueiro Ferreira Filho <janito.vff@gmail.com>
Addresses #2773 (comment)
I changed
zebra/zebra-state/src/service/chain_tip.rs
Line 313 in 44ac067
get_tip_change()
doesn't miss any tip changes.