[wip] SSZ-optimize proof (type 1) #441
Conversation
| type StemStateDiff struct { | ||
| Stem [StemSize]byte `json:"stem"` | ||
| SuffixDiffs SuffixStateDiffs `json:"suffixDiffs"` |
There was a problem hiding this comment.
OK, now makes sense. The PR description has a small mistake that you might want to fix. Delete the SuffixDiffs SuffixStateDiffs json:"suffixDiffs" line since this field is removed.
| SuffixDiffs SuffixStateDiffs `json:"suffixDiffs"` | ||
| Stem [StemSize]byte `json:"stem"` | ||
| Suffixes []byte `json:"suffixes"` | ||
| Current [][]byte `json:"current"` |
There was a problem hiding this comment.
We should remember that a proof verifier must now check that []byte has length 32 which was implicit in the type before.
| copy(ret[i].Suffixes, sd[i].Suffixes) | ||
| ret[i].Current = make([][]byte, len(sd[i].Current)) | ||
| for j := range sd[i].Current { | ||
| if len(sd[i].Current[j]) == 0 { |
There was a problem hiding this comment.
Nit: Remove this empty case, and only leave the else.
| if len(sd[i].Current[j]) == 0 { | ||
|
|
||
| } else { | ||
| copy(ret[i].Current[j], sd[i].Current[j]) |
There was a problem hiding this comment.
I think there's a bug here? We never allocated ret[i].Current[j] so it's an empty slice. The copy will copy the number of bytes being minimum between the two parameters, thus 0.
There was a problem hiding this comment.
Note that in SerializeProof an append is done, which is correct since it will grow as needed compared to this copy.
| if len(sd[i].New[j]) == 0 { | ||
|
|
||
| } else { | ||
| copy(ret[i].New[j], sd[i].New[j]) | ||
| } |
This is the first suggestion by Péter, it replaces the proposed suffix diff format with:
The size improvement compared to the currently proposed method, is significant:
The comparison is a bit unfair, since the library doesn't support optionals and so the encoding for the default method is greatly inflated. But one could argue that if optionals can not be implemented, then this is the correct size of the witness.