Remove all references to children in InternalNode & use uncompressed …

…serialization (#354)

* don't rely on the commitment in HashedNode

change leaf serialization to add the leaf commitment there

fix a few tests

fix: incorrect size computation caused commitment overwrite

fix some unit tests

fix the batch unit test

* declare paths for serialization

* Add CommitmentBytes back in SerializedNode

* fixes from replaying

* fix: deepsource warnings

* tree: avoid memory allocs while collecting paths (#371)

Signed-off-by: Ignacio Hagopian <jsign.uy@gmail.com>

* pbss: serialized points in uncompressed form (#370)

* config: rename serialized point size constant and value to uncompressed form

Signed-off-by: Ignacio Hagopian <jsign.uy@gmail.com>

* switch to uncompressed serialization

Signed-off-by: Ignacio Hagopian <jsign.uy@gmail.com>

---------

Signed-off-by: Ignacio Hagopian <jsign.uy@gmail.com>

* tree: transform first layer to hashed nodes after serializing (#372)

* tree: after serializing transform first layer to hashed nodes

Signed-off-by: Ignacio Hagopian <jsign.uy@gmail.com>

* tree: add comment about temporary code

Signed-off-by: Ignacio Hagopian <jsign.uy@gmail.com>

---------

Signed-off-by: Ignacio Hagopian <jsign.uy@gmail.com>

* tree: make HashedNode non-pointer and fieldless (#373)

* tree: make HashedNode non-pointer and fieldless

Signed-off-by: Ignacio Hagopian <jsign.uy@gmail.com>

* lints

Signed-off-by: Ignacio Hagopian <jsign.uy@gmail.com>

* hashednode: add panic comment

Signed-off-by: Ignacio Hagopian <jsign.uy@gmail.com>

* hashednode: add label in toDot

Signed-off-by: Ignacio Hagopian <jsign.uy@gmail.com>

---------

Signed-off-by: Ignacio Hagopian <jsign.uy@gmail.com>

* rebase fix

Signed-off-by: Ignacio Hagopian <jsign.uy@gmail.com>

---------

Signed-off-by: Ignacio Hagopian <jsign.uy@gmail.com>
Co-authored-by: Ignacio Hagopian <jsign.uy@gmail.com>

conversion.go

@@ -101,12 +101,12 @@ func (n *InternalNode) InsertMigratedLeaves(leaves []LeafNode, resolver NodeReso
 
 		// Look for the appropriate parent for the leaf node.
 		for {
-			if hashedNode, ok := parent.children[ln.stem[parent.depth]].(*HashedNode); ok {
-				serialized, err := resolver(hashedNode.commitment)
+			if _, ok := parent.children[ln.stem[parent.depth]].(HashedNode); ok {
+				serialized, err := resolver(ln.stem[:parent.depth+1])
 				if err != nil {
-					return fmt.Errorf("resolving node %x: %w", hashedNode.commitment, err)
+					return fmt.Errorf("resolving node path=%x: %w", ln.stem[:parent.depth+1], err)
 				}
-				resolved, err := ParseNode(serialized, parent.depth+1, hashedNode.commitment)
+				resolved, err := ParseNode(serialized, parent.depth+1)
 				if err != nil {
 					return fmt.Errorf("parsing node %x: %w", serialized, err)
 				}

debug_test.go

@@ -35,7 +35,7 @@ func TestJSON(t *testing.T) {
 	root.Insert(oneKeyTest, zeroKeyTest, nil)
 	root.Insert(forkOneKeyTest, zeroKeyTest, nil) // Force an internal node in the first layer.
 	root.Insert(fourtyKeyTest, oneKeyTest, nil)
-	root.(*InternalNode).children[152] = &HashedNode{commitment: []byte{1, 2, 3, 4}}
+	root.(*InternalNode).children[152] = HashedNode{}
 	root.Commit()
 
 	output, err := root.(*InternalNode).ToJSON()

encoding.go

@@ -44,15 +44,16 @@ const (
 	nodeTypeOffset = 0
 
 	// Internal nodes offsets.
-	internalBitlistOffset      = nodeTypeOffset + nodeTypeSize
-	internalNodeChildrenOffset = internalBitlistOffset + bitlistSize
+	internalBitlistOffset    = nodeTypeOffset + nodeTypeSize
+	internalCommitmentOffset = internalBitlistOffset + bitlistSize
 
 	// Leaf node offsets.
 	leafSteamOffset        = nodeTypeOffset + nodeTypeSize
 	leafBitlistOffset      = leafSteamOffset + StemSize
-	leafC1CommitmentOffset = leafBitlistOffset + bitlistSize
-	leafC2CommitmentOffset = leafC1CommitmentOffset + SerializedPointCompressedSize
-	leafChildrenOffset     = leafC2CommitmentOffset + SerializedPointCompressedSize
+	leafCommitmentOffset   = leafBitlistOffset + bitlistSize
+	leafC1CommitmentOffset = leafCommitmentOffset + SerializedPointUncompressedSize
+	leafC2CommitmentOffset = leafC1CommitmentOffset + SerializedPointUncompressedSize
+	leafChildrenOffset     = leafC2CommitmentOffset + SerializedPointUncompressedSize
 )
 
 func bit(bitlist []byte, nr int) bool {
@@ -66,27 +67,25 @@ var errSerializedPayloadTooShort = errors.New("verkle payload is too short")
 
 // ParseNode deserializes a node into its proper VerkleNode instance.
 // The serialized bytes have the format:
-// - Internal nodes: <nodeType><bitlist><children...>
-// - Leaf nodes:     <nodeType><stem><bitlist><c1comm><c2comm><children...>
-func ParseNode(serializedNode []byte, depth byte, comm SerializedPointCompressed) (VerkleNode, error) {
+// - Internal nodes: <nodeType><bitlist><commitment>
+// - Leaf nodes:     <nodeType><stem><bitlist><comm><c1comm><c2comm><children...>
+func ParseNode(serializedNode []byte, depth byte) (VerkleNode, error) {
 	// Check that the length of the serialized node is at least the smallest possible serialized node.
-	if len(serializedNode) < nodeTypeSize+bitlistSize {
+	if len(serializedNode) < nodeTypeSize+SerializedPointUncompressedSize {
 		return nil, errSerializedPayloadTooShort
 	}
 
 	switch serializedNode[0] {
 	case leafRLPType:
-		return parseLeafNode(serializedNode, depth, comm)
+		return parseLeafNode(serializedNode, depth)
 	case internalRLPType:
-		bitlist := serializedNode[internalBitlistOffset : internalBitlistOffset+bitlistSize]
-		children := serializedNode[internalNodeChildrenOffset:]
-		return CreateInternalNode(bitlist, children, depth, comm)
+		return CreateInternalNode(serializedNode[internalBitlistOffset:internalCommitmentOffset], serializedNode[internalCommitmentOffset:], depth)
 	default:
 		return nil, ErrInvalidNodeEncoding
 	}
 }
 
-func parseLeafNode(serialized []byte, depth byte, comm SerializedPointCompressed) (VerkleNode, error) {
+func parseLeafNode(serialized []byte, depth byte) (VerkleNode, error) {
 	bitlist := serialized[leafBitlistOffset : leafBitlistOffset+bitlistSize]
 	var values [NodeWidth][]byte
 	offset := leafChildrenOffset
@@ -102,46 +101,48 @@ func parseLeafNode(serialized []byte, depth byte, comm SerializedPointCompressed
 	ln := NewLeafNodeWithNoComms(serialized[leafSteamOffset:leafSteamOffset+StemSize], values[:])
 	ln.setDepth(depth)
 	ln.c1 = new(Point)
-	ln.c1.SetBytesTrusted(serialized[leafC1CommitmentOffset : leafC1CommitmentOffset+SerializedPointCompressedSize])
+
+	// Sanity check that we have at least 3*SerializedPointUncompressedSize bytes left in the serialized payload.
+	if len(serialized[leafCommitmentOffset:]) < 3*SerializedPointUncompressedSize {
+		return nil, fmt.Errorf("leaf node commitments are not the correct size, expected at least %d, got %d", 3*SerializedPointUncompressedSize, len(serialized[leafC1CommitmentOffset:]))
+	}
+
+	ln.c1.SetBytesUncompressed(serialized[leafC1CommitmentOffset:leafC1CommitmentOffset+SerializedPointUncompressedSize], true)
 	ln.c2 = new(Point)
-	ln.c2.SetBytesTrusted(serialized[leafC2CommitmentOffset : leafC2CommitmentOffset+SerializedPointCompressedSize])
+	ln.c2.SetBytesUncompressed(serialized[leafC2CommitmentOffset:leafC2CommitmentOffset+SerializedPointUncompressedSize], true)
 	ln.commitment = new(Point)
-	ln.commitment.SetBytesTrusted(comm)
+	ln.commitment.SetBytesUncompressed(serialized[leafCommitmentOffset:leafC1CommitmentOffset], true)
 	return ln, nil
 }
 
-func CreateInternalNode(bitlist []byte, raw []byte, depth byte, comm SerializedPointCompressed) (*InternalNode, error) {
+func CreateInternalNode(bitlist []byte, raw []byte, depth byte) (*InternalNode, error) {
 	// GetTreeConfig caches computation result, hence
 	// this op has low overhead
-	n := (newInternalNode(depth)).(*InternalNode)
-	indices := indicesFromBitlist(bitlist)
+	node := new(InternalNode)
 
-	if len(raw)/SerializedPointCompressedSize != len(indices) {
+	if len(bitlist) != bitlistSize {
 		return nil, ErrInvalidNodeEncoding
 	}
 
-	freelist := make([]HashedNode, len(indices))
-	for i, index := range indices {
-		freelist[i].commitment = raw[i*SerializedPointCompressedSize : (i+1)*SerializedPointCompressedSize]
-		n.children[index] = &freelist[i]
-	}
-	n.commitment = new(Point)
-	n.commitment.SetBytesTrusted(comm)
-	return n, nil
-}
-
-func indicesFromBitlist(bitlist []byte) []int {
-	indices := make([]int, 0, 32)
+	// Create a HashNode placeholder for all values
+	// corresponding to a set bit.
+	node.children = make([]VerkleNode, NodeWidth)
 	for i, b := range bitlist {
-		if b == 0 {
-			continue
-		}
-		// the bitmap is little-endian, inside a big-endian byte list
 		for j := 0; j < 8; j++ {
 			if b&mask[j] != 0 {
-				indices = append(indices, i*8+j)
+				node.children[8*i+j] = HashedNode{}
+			} else {
+
+				node.children[8*i+j] = Empty(struct{}{})
 			}
 		}
 	}
-	return indices
+	node.depth = depth
+	if len(raw) != SerializedPointUncompressedSize {
+		return nil, ErrInvalidNodeEncoding
+	}
+
+	node.commitment = new(Point)
+	node.commitment.SetBytesUncompressed(raw, true)
+	return node, nil
 }

encoding_test.go

@@ -3,7 +3,7 @@ package verkle
 import "testing"
 
 func TestParseNodeEmptyPayload(t *testing.T) {
-	_, err := ParseNode([]byte{}, 0, SerializedPointCompressed{})
+	_, err := ParseNode([]byte{}, 0)
 	if err != errSerializedPayloadTooShort {
 		t.Fatalf("invalid error, got %v, expected %v", err, "unexpected EOF")
 	}
@@ -21,14 +21,14 @@ func TestLeafStemLength(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
-	if len(ser) != nodeTypeSize+StemSize+bitlistSize+2*SerializedPointCompressedSize {
-		t.Fatalf("invalid serialization when the stem is longer than 31 bytes: %x (%d bytes)", ser, len(ser))
+	if len(ser) != nodeTypeSize+StemSize+bitlistSize+3*SerializedPointUncompressedSize {
+		t.Fatalf("invalid serialization when the stem is longer than 31 bytes: %x (%d bytes != %d)", ser, len(ser), nodeTypeSize+StemSize+bitlistSize+2*SerializedPointUncompressedSize)
 	}
 }
 
 func TestInvalidNodeEncoding(t *testing.T) {
 	// Test a short payload.
-	if _, err := ParseNode([]byte{leafRLPType}, 0, SerializedPointCompressed{}); err != errSerializedPayloadTooShort {
+	if _, err := ParseNode([]byte{leafRLPType}, 0); err != errSerializedPayloadTooShort {
 		t.Fatalf("invalid error, got %v, expected %v", err, errSerializedPayloadTooShort)
 	}
 
@@ -44,7 +44,7 @@ func TestInvalidNodeEncoding(t *testing.T) {
 		t.Fatalf("serializing leaf node: %v", err)
 	}
 	lnbytes[0] = leafRLPType + internalRLPType // Change the type of the node to something invalid.
-	if _, err := ParseNode(lnbytes, 0, SerializedPointCompressed{}); err != ErrInvalidNodeEncoding {
+	if _, err := ParseNode(lnbytes, 0); err != ErrInvalidNodeEncoding {
 		t.Fatalf("invalid error, got %v, expected %v", err, ErrInvalidNodeEncoding)
 	}
 }

go.mod

@@ -2,7 +2,7 @@ module github.com/gballet/go-verkle
 
 go 1.19
 
-require github.com/crate-crypto/go-ipa v0.0.0-20230601170251-1830d0757c80
+require github.com/crate-crypto/go-ipa v0.0.0-20230626131944-6a9b06cf26df
 
 require (
 	golang.org/x/sync v0.1.0 // indirect

go.sum

@@ -1,5 +1,5 @@
-github.com/crate-crypto/go-ipa v0.0.0-20230601170251-1830d0757c80 h1:DuBDHVjgGMPki7bAyh91+3cF1Vh34sAEdH8JQgbc2R0=
-github.com/crate-crypto/go-ipa v0.0.0-20230601170251-1830d0757c80/go.mod h1:gzbVz57IDJgQ9rLQwfSk696JGWof8ftznEL9GoAv3NI=
+github.com/crate-crypto/go-ipa v0.0.0-20230626131944-6a9b06cf26df h1:MV8zGEpmkuAocFqFclIJ3zUz1+6rv0T5QYWD9UDDRVE=
+github.com/crate-crypto/go-ipa v0.0.0-20230626131944-6a9b06cf26df/go.mod h1:gzbVz57IDJgQ9rLQwfSk696JGWof8ftznEL9GoAv3NI=
 github.com/leanovate/gopter v0.2.9 h1:fQjYxZaynp97ozCzfOyOuAGOU4aU/z37zf/tOujFk7c=
 github.com/leanovate/gopter v0.2.9/go.mod h1:U2L/78B+KVFIx2VmW6onHJQzXtFb+p5y3y2Sh+Jxxv8=
 golang.org/x/sync v0.1.0 h1:wsuoTGHzEhffawBOhz5CYhcrV4IdKZbEyZjBMuTp12o=

hashednode.go

@@ -30,69 +30,54 @@ import (
 	"fmt"
 )
 
-type HashedNode struct {
-	commitment  []byte
-	cachedPoint *Point
-}
+type HashedNode struct{}
 
-func (*HashedNode) Insert([]byte, []byte, NodeResolverFn) error {
+func (HashedNode) Insert([]byte, []byte, NodeResolverFn) error {
 	return errInsertIntoHash
 }
 
-func (*HashedNode) Delete([]byte, NodeResolverFn) (bool, error) {
+func (HashedNode) Delete([]byte, NodeResolverFn) (bool, error) {
 	return false, errors.New("cant delete a hashed node in-place")
 }
 
-func (*HashedNode) Get([]byte, NodeResolverFn) ([]byte, error) {
+func (HashedNode) Get([]byte, NodeResolverFn) ([]byte, error) {
 	return nil, errors.New("can not read from a hash node")
 }
 
-func (n *HashedNode) Commit() *Point {
-	if n.commitment == nil {
-		panic("nil commitment")
-	}
-	if n.cachedPoint == nil {
-		n.cachedPoint = new(Point)
-		n.cachedPoint.SetBytesTrusted(n.commitment)
-	}
-	return n.cachedPoint
+func (HashedNode) Commit() *Point {
+	// TODO: we should reconsider what to do with the VerkleNode interface and how
+	//       HashedNode fits into the picture, since Commit(), Commitment() and Hash()
+	//	     now panics. Despite these calls must not happen at runtime, it is still
+	//	     quite risky. The reason we end up in this place is because PBSS came quite
+	//	     recently compared with the VerkleNode interface design. We should probably
+	//	     reconsider splitting the interface or find some safer workaround.
+	panic("can not commit a hash node")
 }
 
-func (n *HashedNode) Commitment() *Point {
-	if n.commitment == nil {
-		panic("nil commitment")
-	}
-	return n.Commit()
+func (HashedNode) Commitment() *Point {
+	panic("can not get commitment of a hash node")
 }
 
-func (*HashedNode) GetProofItems(keylist) (*ProofElements, []byte, [][]byte, error) {
+func (HashedNode) GetProofItems(keylist) (*ProofElements, []byte, [][]byte, error) {
 	return nil, nil, nil, errors.New("can not get the full path, and there is no proof of absence")
 }
 
-func (*HashedNode) Serialize() ([]byte, error) {
+func (HashedNode) Serialize() ([]byte, error) {
 	return nil, errSerializeHashedNode
 }
 
-func (n *HashedNode) Copy() VerkleNode {
-	if n.commitment == nil {
-		panic("nil commitment")
-	}
-	c := &HashedNode{commitment: make([]byte, len(n.commitment))}
-	copy(c.commitment, n.commitment)
-	return c
+func (HashedNode) Copy() VerkleNode {
+	return HashedNode{}
 }
 
-func (n *HashedNode) toDot(parent, path string) string {
-	return fmt.Sprintf("hash%s [label=\"H: %x\"]\n%s -> hash%s\n", path, n.commitment, parent, path)
+func (HashedNode) toDot(parent, path string) string {
+	return fmt.Sprintf("hash%s [label=\"unresolved\"]\n%s -> hash%s\n", path, parent, path)
 }
 
-func (*HashedNode) setDepth(_ byte) {
+func (HashedNode) setDepth(_ byte) {
 	// do nothing
 }
 
-func (n *HashedNode) Hash() *Fr {
-	comm := n.Commitment()
-	hash := new(Fr)
-	toFr(hash, comm)
-	return hash
+func (HashedNode) Hash() *Fr {
+	panic("can not hash a hashed node")
 }

hashednode_test.go

@@ -28,8 +28,7 @@ package verkle
 import "testing"
 
 func TestHashedNodeFuncs(t *testing.T) {
-	fakeCommitment := EmptyCodeHashPoint.Bytes()
-	e := HashedNode{commitment: fakeCommitment[:]}
+	e := HashedNode{}
 	err := e.Insert(zeroKeyTest, zeroKeyTest, nil)
 	if err != errInsertIntoHash {
 		t.Fatal("got nil error when inserting into a hashed node")
@@ -51,7 +50,4 @@ func TestHashedNodeFuncs(t *testing.T) {
 	if _, err := e.Serialize(); err != errSerializeHashedNode {
 		t.Fatal("got nil error when serializing a hashed node")
 	}
-	if hash := e.Hash(); hash == nil {
-		t.Fatal("got nil hash when hashing a hashed node")
-	}
 }

ipa.go

@@ -40,7 +40,7 @@ type (
 )
 
 const (
-	SerializedPointCompressedSize = 32
+	SerializedPointUncompressedSize = 64
 )
 
 func CopyFr(dst, src *Fr) {