-
Notifications
You must be signed in to change notification settings - Fork 2
feat: Add SPDD and DRDD modules with optional historical storage #110
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
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: William Hankins <[email protected]>
Signed-off-by: William Hankins <[email protected]>
Signed-off-by: William Hankins <[email protected]>
I've also included the |
Signed-off-by: William Hankins <[email protected]>
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 good, probably only the incorrect label in the info_span of drdd_state really needs addressing.
…am macro Signed-off-by: William Hankins <[email protected]>
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 good, but check the comment on OrdMap
Signed-off-by: William Hankins <[email protected]>
{ | ||
let mut next = self.get_latest().cloned().unwrap_or_else(OrdMap::new); | ||
|
||
// Update new or changed entries |
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 looks very similar to what's going on in drdd, just a slightly different contains check. For handling rollback state we currently have StateHistory in the common lib that works if your top level per-block state is a an imbl container. It might be nice to add a helper function alongside it for things like this where you have a new state to compare differences with the current one, to apply to an imbl::OrdMap clone of the current state. I can't help but feel that StateHistory might be extendable to work for epochs and retain all snapshots too.
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'll expand StateHistory to handle epoch-based states and add a helper function to compare differences between imbl:OrdMap
s when inserting.
@whankinsiv There are unresolved conflicts now which I can't directly fix - I suspect this has to do with 'cargo fmt' changes in AccountsState. I hope once we have 'cargo fmt' on CI this kind of thing will stop! |
This PR:
spdd_state
module to optionally store stake pool delegation distributions by epoch./spdd
REST endpoint fromaccounts_state
tospdd_state
.caryatid_module_rest_server
to v0.14.0 across applicable modules./spdd
endpoint to accept an optionalepoch
query parameter (Ex:/spdd?epoch=209
).In this implementation, the
/spdd
endpoint is always registered, while state creation and population are determined by thestore-spdd
config flag inomnibus.toml
. Whenstore-spdd
is set tofalse
, the handler receives aNone
state and returns a 503 error: "SPDD storage is disabled by configuration."