Skip to content
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

[wip] SSZ-optimize proof (type 3) #447

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
Draft

[wip] SSZ-optimize proof (type 3) #447

wants to merge 2 commits into from

Conversation

jsign
Copy link
Collaborator

@jsign jsign commented Jul 26, 2024

This PR is grounded on Type 2, but we encapsulate each fields in their own containers.
This should avoid some u32 references as dynamic data, leaving only one per group.

It also changed []byte to [32]byte since the nil cases where already partitioned by Type 2, so those can't happen in Current values.

jsign added 2 commits July 26, 2024 16:11
Signed-off-by: Ignacio Hagopian <[email protected]>
Signed-off-by: Ignacio Hagopian <[email protected]>
switch {
case isInsertion(preLen, postLen):
var newValue [32]byte
copy(newValue[:], proof.PostValues[i])
Copy link
Collaborator Author

@jsign jsign Jul 26, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that we don't have to do the postLen == 32 and all the alignment thing.

If proof.PostValues[i] isn't 32 bytes, then copy will copy whatever amount of bytes there are (in the front of the slice). So, both in aligned and unaligned cases, the right thing will happen.

Same applies to other cases.

)

for _, suffixdiff := range stemstatediff.SuffixDiffs {
if /* len(suffixdiff.NewValue) > 0 - this only works for a slice */ suffixdiff.NewValue != nil {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This previous if (that was kept in Type 2) was removed. This can also be removed in Type 2 as I mentioned in my review in that PR.

Both in Inserts and Updates, we can't have "nil" or empty values in New.

Comment on lines +196 to +203
suffixes := make([]byte, 0, len(ssd.Updates)+len(ssd.Inserts))
for i := range ssd.Updates {
suffixes = append(suffixes, ssd.Updates[i].Suffix)
}
for i := range ssd.Inserts {
suffixes = append(suffixes, ssd.Inserts[i].Suffix)
}
sort.Slice(suffixes, func(i, j int) bool { return suffixes[i] < suffixes[j] })
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think in Type 2 there might be a bug since it's only inserting Updates, but prob we need both Updates+Inserts as I did here?
I also sort them since I'd guess that's expected.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that's possible, I'm only focusing on the SSZ encoding at the moment. We'll pick the best method and fix it if need be.

@jsign jsign requested a review from gballet July 26, 2024 19:28
ret[i].Updates = make([]UpdateDiff, len(sd[i].Updates))
for j := range sd[i].Updates {
ret[i].Updates[j].Suffix = sd[i].Updates[j].Suffix
ret[i].Updates[j].Current = sd[i].Updates[j].Current
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we stop having []byte, many of if len(xxx) == 0 cases and similar are gone. We can always copy-by-value.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants