-
Notifications
You must be signed in to change notification settings - Fork 75
feat: add "zkvm" flag that reduces VariableList to a manageable size by zkVMs #567
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
KolbyML
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.
Left some feedback
|
Having a long standing branch is a pain. Given that Beam chain client itself is a prototype, I think it's okay to merge it. But this is apparently a tech debt. This issue is coming from After we become able to run Ream in 32-bit zkVMs, I think it's a good time to work on a proper solution. |
jihoonsong
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.
Great work!
KolbyML
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.
I am good with this change
What are you trying to achieve?
This PR adds "zkvm" feature flag so that external programs can config ream codebase in the way that can be run on zkVMs.
In this PR, the feature flag would drop
VariableListthat are 240 in size down to 229 when enabled, so that 32-bit zkVMs can work with ourBeaconState.See ReamLabs/consenzero-bench#24 for how this feature flag is used.
How was it implemented/fixed?
The BeaconState contains some "zkvm" features where 32-bit zkVMs would fail on constructing a
VariableListlarger than 232 size (i.e. 240). When "zkvm" feature is enabled, it would construct the BeaconState with 229 list instead.To remain conformant with
consensus-specs, this feature needs to be used with the modifiedssz_typescrate at https://github.com/ReamLabs/ssz_types/tree/magic-extended-list where the crate would detect 229 as a magic number when computing the root hash, and it will compute as a 240 list root instead.Tbh I'm not sure I actually want to add this to ream's main codebase, but it's needed to run
ream-consensuson zkVMs with correct results. If it's not ideal to merge, I'm happy to keep it as a long runningzkvmbranch on ream as well. Happy to discuss this and also in case anyone has a better idea to manage these differences.To-Do