-
Notifications
You must be signed in to change notification settings - Fork 12
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
feat: add reorgs support + indexer restructuring #53
Conversation
…rate the indexer's checkpoint slicing logic within the synchronizer
…er function for better clarity
… and PUT request macros
…on issue There's a potential race condition issue when handling chain reorganizations by subscribing to block reorg events. This situation arises when new blocks from the newly established canonical chain are indexed prior to the processing of the reorg event. As a result, these blocks might be incorrectly labeled, leading to inconsistencies in our indexing process.
3f75e47
to
7f12bd7
Compare
b1a3d44
to
17f5d0c
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 have conducted an initial review of the changes. I still need to cover some important sections on the synchronizer. Anyways I wanted to provide an initial round of feedback.
The refactor of the code has made it more robust with the addition of the synchronizer, which does a great job dividing the sync in two steps. Well done!
Co-authored-by: Gabi <gabrielpk.18@gmail.com>
… part of the syncing process.
462d998
to
3dc2c9a
Compare
135abfc
to
605c242
Compare
…'s and synchronizer's error logging
605c242
to
e926869
Compare
e38af2f
to
a82000a
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.
Fantastic job! I've noticed considerable improvements since my last review. I'm particularly impressed by how you've managed to split the synchronization into two separate processes, as we discussed. The addition of real-time sync significantly enhances the usability of Blobscan, allowing users to engage with it immediately without having to wait for the entire chain to synchronize.
Furthermore, the updates to the error messages are a substantial improvement. They now more effectively explain which dependency is causing an issue, which is super helpful.
I have very few additional suggestions to offer. Well done!
It resolves #19
It resolves #46
It resolves #49
It depends on the following API's PR
This PR contains the following refactoring:
It also adds support for block reorgs.