-
Notifications
You must be signed in to change notification settings - Fork 88
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
Refactor chain state persistency #1049
Conversation
ec4da37
to
cba1aab
Compare
Transactions CostsSizes and execution budgets for Hydra protocol transactions. Note that unlisted parameters are currently using
Script summary
Cost of Init Transaction
Cost of Commit TransactionThis is using ada-only outputs for better comparability.
Cost of CollectCom Transaction
Cost of Close Transaction
Cost of Contest Transaction
Cost of Abort TransactionSome variation because of random mixture of still initial and already committed outputs.
Cost of FanOut TransactionInvolves spending head output and burning head tokens. Uses ada-only UTxO for better comparability.
|
5536802
to
be45db0
Compare
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.
🎉
5167136
to
2debe86
Compare
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.
Before, the chainState was really abstract to the HeadLogic but that came with the overhead of having to store the full chainState history with every chain originated event. I'm not sure how heavy this overhead was but I can see how this PR removes it and that's cool.
So now, we do not store the chainState full history every time but it comes with the HeadlLogic having to know about the fact that there is a notion of chainState history and the headLogic know has to deal with it and, for instance, call this pushNewState
function.
That's a shame the HeadLogic has to have this knowledge but I understand that there is a trade-off here and I'm fine with this way of dealing with it 👍
f521683
to
49528af
Compare
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 like the direction this is going. The fact that we do not need the full history on every chain event is great.
I do have two major points though, which are worth looking into.
As we are refactoring here, I feel like we should be pushing further and directly to a better situation. Details below 👇
cb00516
to
cf7a42a
Compare
caf99d3
to
5e5c466
Compare
baedbb6
to
354ead8
Compare
Since ChainStateType is not recursive anymore, the HeadState now keeps a non-empty list of ChainStateType instead. This means, now the HeadLogic has to collect the new ChainStateType during 'aggregate'. Finally, the LocalChainState now has to maintain a non-empty list instead in order to implement 'rollback'.
Introduce 'history' handle to get all chain changed events. Note: in the future we might want to change the interface to be a stream. This new handle was needed in order to be able to resume the LocalChainState.
We were aggregating wrong the Rollback observation by appending the rolledBack point to the previous history, but we needed to override it instead.
Add helper functions to avoid scattering the code all over the place. Also made some renaming to keep things cohesive
This removes the need to keep the entire history as part of the HeadState, keeping the persistence size at minimum.
Instead, we made a generic rollbackHistory function, that uses the chainStateSlot to know until which point in time to rollback from history. This brings the benefit of making ChainRolledBack very lightweight, reducing the persistence size.
This avoids the need to expose the underlying structure of it.
The fact that the golden tests are only removing previous and still pass, is proof that this is in fact a backward compatible change.
Make it depend on the type family instead of the concrete instance. Also remove unused imports and minor formatting changes.
This removes the need of passing the initial chain state around and allows us to use the LocalChainState handle on the BehaviorSpec.
354ead8
to
fe14efe
Compare
🪚 Remove previous attribute from ChainStateAt.
🪚 Since ChainStateType is not recursive anymore, the HeadState now keeps a non-empty list of ChainStateType instead.
🪚 This means, now the HeadLogic has to collect the new ChainStateType during 'aggregate'.
🪚 Finally, the LocalChainState now has to maintain a non-empty list instead in order to implement 'rollback'.
🪚 As a consequence, now the persistent state is more lightweight and easier to read and work with.