Conversation
|
Converting back to draft to address Jun's comments when I wake up. |
|
If you are using usize for zkVM's as it would be a u32, we still shouldn't use usize we should use u32 and define it in the leanSpecs |
| impl Vote { | ||
| pub fn compute_hash(&self) -> B256 { | ||
| let serialized = serde_json::to_string(self).unwrap(); | ||
| B256::from_slice(&hash(serialized.as_bytes())) | ||
| } | ||
| } |
There was a problem hiding this comment.
The hash of Vote is computed by calling .tree_root_hash() on the object like how it is done in the beacon chain right?, I don't think we need this function
There was a problem hiding this comment.
I guess this is a ported function from the original code (by Vitalik). Of course it will be deleted after using SSZ here.
There was a problem hiding this comment.
I've resolved all issues with SSZ compatibility and so replaced compute_hash() with TreeHash now.
|
Generally |
syjn99
left a comment
syjn99
left a comment
There was a problem hiding this comment.
Some nits and concerns! Thanks
KolbyML
left a comment
KolbyML
left a comment
There was a problem hiding this comment.
Could we remove the rest of the unwrap()'s as well, although I think we should just use anyhow, as it isn't much work
|
Remaining: move to anyhow |
@KolbyML I tried moving |
syjn99
left a comment
syjn99
left a comment
There was a problem hiding this comment.
I only have some code styling concerns, and am mostly happy with the content here. Thanks O!
Approved proactively from my side
3 participants
What was wrong?
As part of #670, this PR brings in the 3SF-mini implementation in rust at https://github.com/ReamLabs/rust-3sf into ream repo under
ream-consensus-leancrate.This PR modified the 3SF-mini code only enough so that the build passes, so that we can start parallelize more work e.g. #671, etc. Reason being there're more changes needed to complete #670, e.g.
VariableListdoes not support item removal but we need it for tracking justifications.Options that are not supported by SSZHow was it fixed?
Stakerfromcommon/consensus/leantocommon/lean_chainjustifications: HashMap<Hash, Vec<bool>>as a flatBitList#[derive(Encode, Decode, TreeHash)]to all containersanyhowfor error handlingBlockandVoteBlockByRootStateis super heavy. It's storinghistorical_block_hashesandjustified_slotsfor the entire chain history. Can it be trimmed? (maybe not yet for devnet-0)To-Do