-
Notifications
You must be signed in to change notification settings - Fork 141
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
State sync 1 - Blocks backfilling option #353
Conversation
56e20ff
to
48a940d
Compare
ee2c85b
to
4205d7f
Compare
plugin/evm/syncervm_client.go
Outdated
BackfillBlocksEnabled(ctx context.Context) (ids.ID, uint64, error) | ||
BackfillBlocks(ctx context.Context, blocks [][]byte) (ids.ID, uint64, error) |
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.
not sure where these methods should be implemented. They are part of the StateSync interface so this is the natural place; but their implementation needs the full vm (to parse and store blocks) which is not passed to the state sync client.
For now I end up providing functions to parse and get blocks via the config, not sure if we have a different canonical way to do this in coreth
plugin/evm/syncervm_client.go
Outdated
break | ||
} | ||
|
||
if err := blk.Accept(ctx); err != nil { |
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.
This is most likely wrong. Accepting a past block seems to mess up a whole bunch of metadata.
@darioush I'd love a feedback on how the indexing of past block should be handled (brand new methods? Upgrade block.Accept to handle past blocks? something else?).
7d716b6
to
b7d5280
Compare
b7d5280
to
e0ae369
Compare
Why this should be merged
Counterpart of State sync - Blocks backfilling option
How this works
Missing stuff:
How this was tested
Extra UTs + Fuji run (ongoing)