Don't update mempool in handle_new_tip if the actual chainstate tip differs #1507
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
ImplOfAnImpl
requested review from
erubboli,
iljakuklic,
TheQuantumPhysicist and
azarovh
as code owners
ImplOfAnImpl
commented
Jan 26, 2024
mempool/src/pool/reorg.rs
Outdated
Comment on lines
165
to
177
| // We still need to update the current tx_verifier tip | ||
| let mut old_transactions = mempool.reset(); | ||
| if old_transactions.next().is_some() { | ||
| log::warn!("Discarding mempool transactions during IBD"); | ||
| log::warn!("Discarding mempool transactions during IBD or because the tip has changed"); | ||
| } | ||
| return Ok(()); |
There was a problem hiding this comment.
Btw, is this needed when the tip is out of sync? Why was it needed in the ibd case?
There was a problem hiding this comment.
I think in the IBD case, if the IBD correctly behaves as a latch, it should not happen. I just found it a bit risky to go for a full assertion. I would say dropping transactions from mempool is not appropriate when mempool is temporarily out of sync. However, when mempool catches up, it should reset() and reverify to see which transactions still apply at the current tip.
There was a problem hiding this comment.
It turned out that some functional tests do depend on this "mempool.reset" during ibd, so I left this call under "if is_ibd" and wrote a comment.
TheQuantumPhysicist
approved these changes
Jan 26, 2024
ImplOfAnImpl
force-pushed
the
dont_update_mempool_if_tip_differs
branch
from
7c23c17 to
9f0b495
Compare
iljakuklic
approved these changes
Jan 26, 2024
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.
No description provided.