Skip to content

Conversation

@manuelmauro
Copy link
Contributor

Description

Fixes a race condition where intermediate blocks during reorgs were not being notified to eth_subscribe("newHeads") subscribers (issue reported on Moonbeam).

The issue was that MappingSyncWorker determined is_new_best by querying client.info().best_hash at sync time, not at import time. During rapid reorgs, the best hash could change between import and sync, causing intermediate blocks to incorrectly have is_new_best=false and be filtered out by EthPubSub.

Changes:

  • Add best_at_import HashSet to track blocks that were is_new_best at the time of their BlockImportNotification
  • Capture notification.is_new_best from import notifications
  • Use import-time status when available, fall back to current best hash check for blocks synced during catch-up

…ions

Fix a race condition where intermediate blocks during reorgs were not
being notified to eth_subscribe("newHeads") subscribers.

The issue was that MappingSyncWorker determined is_new_best by querying
client.info().best_hash at sync time, not at import time. During rapid
reorgs, the best hash could change between import and sync, causing
intermediate blocks to incorrectly have is_new_best=false and be
filtered out by EthPubSub.

Changes:
- Add best_at_import HashSet to track blocks that were is_new_best
  at the time of their BlockImportNotification
- Capture notification.is_new_best from import notifications
- Use import-time status when available, fall back to current best
  hash check for blocks synced during catch-up
Copy link
Collaborator

@librelois librelois left a comment

Choose a reason for hiding this comment

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

It sees that the new HashSet best_at_import is not properly pruned, this imply potential infinite growth.
Can you rethink the implementation to make sure the list best_at_import is bounded and properly pruned?

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.

2 participants