-
Notifications
You must be signed in to change notification settings - Fork 66
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
Use skiplists to save on the bitmap size when compressing leaves #454
base: jsign-type-3
Are you sure you want to change the base?
Changes from 4 commits
5444cd1
f40bb5a
3b15f78
fe8246c
ee9413f
4deabaa
a41ce08
12343ee
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -56,12 +56,14 @@ const ( | |
leafC1CommitmentOffset = leafCommitmentOffset + banderwagon.UncompressedSize | ||
leafC2CommitmentOffset = leafC1CommitmentOffset + banderwagon.UncompressedSize | ||
leafChildrenOffset = leafC2CommitmentOffset + banderwagon.UncompressedSize | ||
leafBalanceSize = 32 | ||
leafNonceSize = 8 | ||
leafSkipListCOffset = nodeTypeSize + StemSize | ||
leafSkipListC1Offset = leafSkipListCOffset + banderwagon.UncompressedSize | ||
leafSkipListC2Offset = leafSkipListC1Offset + banderwagon.UncompressedSize | ||
leafBasicDataSize = 32 | ||
leafSlotSize = 32 | ||
leafValueIndexSize = 1 | ||
singleSlotLeafSize = nodeTypeSize + StemSize + 2*banderwagon.UncompressedSize + leafValueIndexSize + leafSlotSize | ||
eoaLeafSize = nodeTypeSize + StemSize + 2*banderwagon.UncompressedSize + leafBalanceSize + leafNonceSize | ||
eoaLeafSize = nodeTypeSize + StemSize + 2*banderwagon.UncompressedSize + leafBasicDataSize | ||
) | ||
|
||
func bit(bitlist []byte, nr int) bool { | ||
|
@@ -94,6 +96,8 @@ func ParseNode(serializedNode []byte, depth byte) (VerkleNode, error) { | |
return parseEoAccountNode(serializedNode, depth) | ||
case singleSlotType: | ||
return parseSingleSlotNode(serializedNode, depth) | ||
case skipListType: | ||
return parseSkipList(serializedNode, depth) | ||
default: | ||
return nil, ErrInvalidNodeEncoding | ||
} | ||
|
@@ -135,17 +139,58 @@ func parseLeafNode(serialized []byte, depth byte) (VerkleNode, error) { | |
return ln, nil | ||
} | ||
|
||
func parseSkipList(serialized []byte, depth byte) (VerkleNode, error) { | ||
var values [NodeWidth][]byte | ||
offset := leafStemOffset + StemSize + 3*banderwagon.UncompressedSize // offset in the serialized payload | ||
valueIdx := 0 // Index of the value being deserialized | ||
|
||
// shortcut: the leaf is full and so both values are 0. | ||
if serialized[offset] == 0 && serialized[offset+1] == 0 { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this worth it? If the node is full, doesn't sound we should serialize with skip list really. Sounds wasteful. |
||
for i := 0; i < 256; i++ { | ||
values[i] = serialized[offset : offset+leafSlotSize] | ||
offset += leafSlotSize | ||
} | ||
} else { | ||
for valueIdx < NodeWidth && offset < len(serialized) { | ||
rangecount := serialized[offset+1] | ||
gapsize := serialized[offset] | ||
valueIdx += int(gapsize) | ||
offset += 2 | ||
for i := 0; i < int(rangecount); i++ { | ||
values[valueIdx] = serialized[offset : offset+leafSlotSize] | ||
offset += leafSlotSize | ||
valueIdx++ | ||
} | ||
} | ||
} | ||
ln := NewLeafNodeWithNoComms(serialized[leafStemOffset:leafStemOffset+StemSize], values[:]) | ||
ln.setDepth(depth) | ||
ln.c1 = new(Point) | ||
|
||
// Sanity check that we have at least 3*banderwagon.UncompressedSize bytes left in the serialized payload. | ||
if len(serialized[leafSkipListCOffset:]) < 3*banderwagon.UncompressedSize { | ||
return nil, fmt.Errorf("leaf node commitments are not the correct size, expected at least %d, got %d", 3*banderwagon.UncompressedSize, len(serialized[leafSkipListCOffset:])) | ||
} | ||
|
||
if err := ln.c1.SetBytesUncompressed(serialized[leafC1CommitmentOffset:leafSkipListC2Offset], true); err != nil { | ||
return nil, fmt.Errorf("setting c1 commitment: %w", err) | ||
} | ||
ln.c2 = new(Point) | ||
if err := ln.c2.SetBytesUncompressed(serialized[leafSkipListC2Offset:leafSkipListC2Offset+banderwagon.UncompressedSize], true); err != nil { | ||
return nil, fmt.Errorf("setting c2 commitment: %w", err) | ||
} | ||
ln.commitment = new(Point) | ||
if err := ln.commitment.SetBytesUncompressed(serialized[leafSkipListCOffset:leafSkipListC1Offset], true); err != nil { | ||
return nil, fmt.Errorf("setting commitment: %w", err) | ||
} | ||
return ln, nil | ||
} | ||
|
||
func parseEoAccountNode(serialized []byte, depth byte) (VerkleNode, error) { | ||
var values [NodeWidth][]byte | ||
offset := leafStemOffset + StemSize + 2*banderwagon.UncompressedSize | ||
values[0] = zero32[:] // 0 version | ||
values[1] = serialized[offset : offset+leafBalanceSize] // balance | ||
var nonce [32]byte | ||
offset += leafBalanceSize | ||
copy(nonce[:leafNonceSize], serialized[offset:offset+leafNonceSize]) | ||
values[2] = nonce[:] // nonce | ||
values[3] = EmptyCodeHash[:] | ||
values[4] = zero32[:] // 0 code size | ||
values[0] = serialized[offset : offset+leafBasicDataSize] // basic data | ||
values[1] = EmptyCodeHash[:] | ||
ln := NewLeafNodeWithNoComms(serialized[leafStemOffset:leafStemOffset+StemSize], values[:]) | ||
ln.setDepth(depth) | ||
ln.c1 = new(Point) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -29,6 +29,7 @@ import ( | |
"bytes" | ||
"errors" | ||
"fmt" | ||
"slices" | ||
"sort" | ||
|
||
ipa "github.com/crate-crypto/go-ipa" | ||
|
@@ -399,14 +400,14 @@ func DeserializeProof(vp *VerkleProof, statediff StateDiff) (*Proof, error) { | |
k[StemSize] = ins.Suffix | ||
keys = append(keys, k[:]) | ||
prevalues = append(prevalues, nil) | ||
postvalues = append(postvalues, ins.New[:]) | ||
postvalues = append(postvalues, slices.Clone(ins.New[:])) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This, and line 410, are the fixes for the test that was broken. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Weird, so this means that There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. the area that is used to store the location of the iterator is reused over and over, and if we pass a reference to it, it will change as the iterator progress. This is not the first time we fall for this, and probably not the last time either, it's quite subtle and easy to forget. |
||
} | ||
for _, rd := range stemdiff.Reads { | ||
var k [32]byte | ||
copy(k[:StemSize], stemdiff.Stem[:]) | ||
k[StemSize] = rd.Suffix | ||
keys = append(keys, k[:]) | ||
prevalues = append(prevalues, rd.Current[:]) | ||
prevalues = append(prevalues, slices.Clone(rd.Current[:])) | ||
postvalues = append(postvalues, nil) | ||
} | ||
for _, mi := range stemdiff.Missing { | ||
|
Original file line number | Diff line number | Diff line change | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -170,6 +170,7 @@ const ( | |||||||||||
leafType byte = 2 | ||||||||||||
eoAccountType byte = 3 | ||||||||||||
singleSlotType byte = 4 | ||||||||||||
skipListType byte = 8 | ||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this is set in bit order, because I'm hoping to mix encodings in the future. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we could change it to 4, no long-term db has been using this. |
||||||||||||
) | ||||||||||||
|
||||||||||||
type ( | ||||||||||||
|
@@ -1775,35 +1776,42 @@ func (n *LeafNode) serializeLeafWithUncompressedCommitments(cBytes, c1Bytes, c2B | |||||||||||
bitlist [bitlistSize]byte | ||||||||||||
isEoA = true | ||||||||||||
count, lastIdx int | ||||||||||||
gapcount int | ||||||||||||
gaps [32]struct { | ||||||||||||
Skip byte // How many slots to skip before the next range | ||||||||||||
Count int // Size of the next range | ||||||||||||
gballet marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||
} | ||||||||||||
) | ||||||||||||
for i, v := range n.values { | ||||||||||||
if v != nil { | ||||||||||||
count++ | ||||||||||||
lastIdx = i | ||||||||||||
gaps[gapcount].Count++ | ||||||||||||
|
||||||||||||
setBit(bitlist[:], i) | ||||||||||||
children = append(children, v...) | ||||||||||||
if padding := emptyValue[:LeafValueSize-len(v)]; len(padding) != 0 { | ||||||||||||
children = append(children, padding...) | ||||||||||||
} | ||||||||||||
} else { | ||||||||||||
if gaps[gapcount].Skip == 255 { | ||||||||||||
panic("empty leaf node") | ||||||||||||
} | ||||||||||||
if i > 0 && n.values[i-1] != nil { | ||||||||||||
gapcount++ | ||||||||||||
} | ||||||||||||
gaps[gapcount].Skip++ | ||||||||||||
} | ||||||||||||
|
||||||||||||
// Check for an EOA | ||||||||||||
if isEoA { | ||||||||||||
switch i { | ||||||||||||
case 0: | ||||||||||||
// Version should be 0 | ||||||||||||
isEoA = v != nil && bytes.Equal(v, zero32[:]) | ||||||||||||
case 1: | ||||||||||||
// Balance should not be nil | ||||||||||||
// Basic data should not be nil | ||||||||||||
isEoA = v != nil | ||||||||||||
case 2: | ||||||||||||
// Nonce should have its last 24 bytes set to 0 | ||||||||||||
isEoA = v != nil && bytes.Equal(v[leafNonceSize:], zero24[:]) | ||||||||||||
case 3: | ||||||||||||
case 1: | ||||||||||||
// Code hash should be the empty code hash | ||||||||||||
isEoA = v != nil && bytes.Equal(v, EmptyCodeHash[:]) | ||||||||||||
case 4: | ||||||||||||
// Code size must be 0 | ||||||||||||
isEoA = v != nil && bytes.Equal(v, zero32[:]) | ||||||||||||
default: | ||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is the EoA fix for Nyota. |
||||||||||||
// All other values must be nil | ||||||||||||
isEoA = v == nil | ||||||||||||
|
@@ -1830,8 +1838,31 @@ func (n *LeafNode) serializeLeafWithUncompressedCommitments(cBytes, c1Bytes, c2B | |||||||||||
copy(result[leafStemOffset:], n.stem[:StemSize]) | ||||||||||||
copy(result[leafStemOffset+StemSize:], c1Bytes[:]) | ||||||||||||
copy(result[leafStemOffset+StemSize+banderwagon.UncompressedSize:], cBytes[:]) | ||||||||||||
copy(result[leafStemOffset+StemSize+2*banderwagon.UncompressedSize:], n.values[1]) // copy balance | ||||||||||||
copy(result[leafStemOffset+StemSize+2*banderwagon.UncompressedSize+leafBalanceSize:], n.values[2][:leafNonceSize]) // copy nonce | ||||||||||||
copy(result[leafStemOffset+StemSize+2*banderwagon.UncompressedSize:], n.values[0]) // copy basic data | ||||||||||||
case gapcount < 16: | ||||||||||||
// If there are less than 16 gaps, it's worth using skiplists | ||||||||||||
result = make([]byte, 1, nodeTypeSize+StemSize+bitlistSize+3*banderwagon.UncompressedSize+len(children)) | ||||||||||||
result[0] = skipListType | ||||||||||||
result = append(result, n.stem[:StemSize]...) | ||||||||||||
result = append(result, cBytes[:]...) | ||||||||||||
result = append(result, c1Bytes[:]...) | ||||||||||||
result = append(result, c2Bytes[:]...) | ||||||||||||
var leafIdx int | ||||||||||||
for _, gap := range gaps { | ||||||||||||
if gap.Count == 0 { | ||||||||||||
break // skip the last gap as nothing follows | ||||||||||||
} | ||||||||||||
Comment on lines
+1863
to
+1866
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could we do:
and avoid There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. not completely, because if |
||||||||||||
result = append(result, gap.Skip) | ||||||||||||
leafIdx += int(gap.Skip) | ||||||||||||
result = append(result, byte(gap.Count)) | ||||||||||||
Comment on lines
+1868
to
+1869
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm thinking there can't be a 0-length range, so we should encode
Suggested change
|
||||||||||||
for i := 0; i < gap.Count; i++ { | ||||||||||||
if len(n.values[leafIdx]) != 32 { | ||||||||||||
panic(fmt.Sprintf("%x", n.values[leafIdx])) | ||||||||||||
} | ||||||||||||
result = append(result, n.values[leafIdx]...) | ||||||||||||
leafIdx++ | ||||||||||||
} | ||||||||||||
} | ||||||||||||
default: | ||||||||||||
result = make([]byte, nodeTypeSize+StemSize+bitlistSize+3*banderwagon.UncompressedSize+len(children)) | ||||||||||||
result[0] = leafType | ||||||||||||
|
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.
Nit:
leafSkipListC0Offset