-
Notifications
You must be signed in to change notification settings - Fork 8
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
Investigate all uses of canonical_header_hash #251
Comments
I wonder how feasible it would be to modify the existing postgres queries so that they only apply their |
This way we could continue to use it to weigh canonicity in the region of the chain that is still subject to reorgs (2 epochs, each with 32 slots-although they don't necessarily have to populated so not a perfect map to 64 execution layer blocks), and below that cutoff we have a mechanism for marking/cleaning out non-canon data. |
If we are okay with no longer persisting non-canon blocks for analytical purposes, we might be able to simply add a UC to In this context, we would no longer use The statediffing service simply subscribes to a stream of latest blocks from geth's In this later case where we don't rely on a DB constraint we need additional logic in the statediffing service to track when a block coming off the feed is a reorg block (the feed doesn't signal this itself). Since reorgs can't go below 2 epochs from head, I think one way to do this would be to maintain a map in memory of the latest ~64 This all relies on the assumption that geth is handling reorgs as expected. |
Somewhat related: https://github.com/vulcanize/ops/issues/46#issuecomment-1474229179 we haven't seen any reorgs post-merge that are longer than 1 block. |
Since we can't have FKs in place in timescaleDB, so cannot cascade the UPSERT to remove all linked rows, relying on the unique constraint doesn't work. It would leave non-canonical records in the other tables. |
My gut thought is to not upsert, but instead retain the "dup" rows, but have a field to denote the most favored row. |
Fixed. |
It is very easy to shoot yourself in the foot when using canonical_header_hash(), such that it gets executed in a nested loop thousands or millions of times.
We need to make sure that all current uses are efficient, and if they are not, either fix them, or make a serious adjustment such that we no longer need canonical_header_hash() at all, for example by marking canonical/non-canonical blocks rather than by executing a fn.
The latter would be ideal long term, because we don't really want to rely on complicated DB functions, since they are very difficult to observe or debug, but how urgent that change is would depend at least in part on determining how much of an issue it is at this time.
The text was updated successfully, but these errors were encountered: