-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Add snapshot processing for all astrid modes #12339
Conversation
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.
🚀
mu sync.Mutex | ||
on chan struct{} | ||
state bool | ||
inited bool |
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.
Typo?
inited bool | |
initiated bool |
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 means init has been called do you want me to change init to initiated, its a well known shorthand.
borSn *BorRoSnapshots | ||
sn *RoSnapshots | ||
borSn *heimdall.RoSnapshots | ||
borBridgeStore bridge.Store |
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.
Are these stores being added here just for the sake of code deduplication?
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.
These are being added so that temporarily block_reader can retain its interface to the stage code. Doing it like this allows us to test the functionality in the bor specific code does not break the current stage code - hence it proves that it running successfully.
Once we remove the stages all of the bor specific code - bridge snapshots etc can be removed from here.
CalculateSprintLength(number uint64) uint64 | ||
} | ||
|
||
func NewSnapshotStore(base Store, snapshots *heimdall.RoSnapshots, sprintLengthCalculator sprintLengthCalculator) *snapshotStore { |
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.
func NewSnapshotStore(base Store, snapshots *heimdall.RoSnapshots, sprintLengthCalculator sprintLengthCalculator) *snapshotStore { | |
func NewSnapshotStore(database Store, snapshots *heimdall.RoSnapshots, sprintLengthCalculator sprintLengthCalculator) *snapshotStore { |
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.
Why assume that the base store is a database ?
I can change it but it seems logically incorrect - if we want to name it after the type shouldn't it just be store ?
This PR allows stand alone astrid to participate in snapshot production.
It includes the following changes:
The current state of the types which manage bor persistence are as follows:
These changes have been tested by running amoy from scratch in various modes:
I have also tested removing chaindata (& heimdall & polygon-bridge) dbs and restarting as well as removing locally produced snapshot files.
In all cases for my testing amoy startes, syncs and runs at the tip.