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

[Bug] MempoolManager.new_peak() fails in case of a re-org where the new tip is not a transaction block #17186

Closed
madMAx43v3r opened this issue Jan 2, 2024 · 5 comments
Assignees
Labels
bug Something isn't working

Comments

@madMAx43v3r
Copy link

What happened?

In case of a re-org, MempoolManager.new_peak() is only called with the new peak, however the function early exits if the given peak is not a transaction block:

if new_peak.is_transaction_block is False:

As such the mempool is not updated correctly in case of a re-org where the new peak is not a transaction block. Only at the next transaction block it will be corrected.

Version

master

What platform are you using?

Linux

What ui mode are you using?

CLI

Relevant log output

No response

@madMAx43v3r madMAx43v3r added the bug Something isn't working label Jan 2, 2024
@wjblanke
Copy link
Contributor

wjblanke commented Jan 3, 2024

Thanks Arvid will take a look

@arvidn
Copy link
Contributor

arvidn commented Jan 3, 2024

It's not immediately obvious what the best approach to fixing this is. Maybe we could back-track to find the most recent transaction block to see if it matches the current peak.

Another approach, that might be simpler, would be to update self.peak every time we see a block, including non-transaction blocks. That way, a (non-tx) block that doesn't follow in the chain could trigger the mempool reorg

@arvidn
Copy link
Contributor

arvidn commented Jan 20, 2024

the mempool doesn't have the ability to traverse the blockchain. If it gets a non-tx block that's disconnected from its current peak, it can't step back to find the most recent transaction block.

I think this issue has to be fixed in full_node.py and make sure it only passes transaction blocks to mempool_manager.new_peak()

@arvidn
Copy link
Contributor

arvidn commented Jan 20, 2024

I'm addressing this issue here: #17370

@wjblanke
Copy link
Contributor

Thanks for the report!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants