-
Notifications
You must be signed in to change notification settings - Fork 5
Ajw/389 bootstrap spo module #458
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
Conversation
- Read straight to common crate types in streaming snapshot - Remove minicbor encoding/decoding from core common types for now - Remove decoding code for ledger state module
sandtreader
left a comment
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.
Looks like a great general pattern - nice that we can decode things like IP addresses directly, and that SPOState's state is literally just the bootstrap message content.
lowhung
left a comment
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.
So clean! Nice to see so much code removal too.
| ); | ||
|
|
||
| // pool_registration (third element) | ||
| let pools: VMap<PoolId, PoolParams> = decoder |
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.
Nice to see all this ugly code go away.
| let start = Instant::now(); | ||
|
|
||
| match parser.parse(&mut callbacks) { | ||
| match parser.parse(&mut callbacks, NetworkId::Mainnet) { |
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.
Pulling the networkid out is nice.
buddhisthead
left a comment
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.
Really nice work, Alex. Thanks for cleaning up all of my gross decoding! This looks great. I ran the test and verified it still works :-)
Description
This PR modifies the streaming snapshot reader to pass acropolis common types to the pools callback and reworks the old snapshot handling code to work with the new bootstrap publisher.
Related Issue(s)
Implements #389.
How was this tested?
From real snapshots and examining the /pools/ endpoint.
Checklist
Impact / Side effects
Reviewer notes / Areas to focus
The rework to the streaming reader would be good to receive feedback on since it's the main part of this PR and may be used as a future pattern for other bootstrap modules