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

remove panics in StemFromBytes and FromLEBytes #366

Merged
merged 4 commits into from
Jun 29, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion config_ipa.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,9 @@ func GetConfig() *Config {
values := make([][]byte, NodeWidth)
values[CodeHashVectorPosition] = emptyHashCode
var c1poly [NodeWidth]Fr
fillSuffixTreePoly(c1poly[:], values[:NodeWidth/2])
if _, err := fillSuffixTreePoly(c1poly[:], values[:NodeWidth/2]); err != nil {
panic(err)
}
Comment on lines +88 to +90
Copy link
Collaborator

Choose a reason for hiding this comment

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

Asking just in case: deferring GetConfig() to return an error?

Copy link
Member Author

Choose a reason for hiding this comment

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

nah, because if you get an error at this stage, there's no point in continuing (and also it would make a huge diff as it's called everywhere)

EmptyCodeHashPoint = *cfg.CommitToPoly(c1poly[:], 0)
EmptyCodeHashFirstHalfValue = c1poly[EmptyCodeHashFirstHalfIdx]
EmptyCodeHashSecondHalfValue = c1poly[EmptyCodeHashSecondHalfIdx]
Expand Down
24 changes: 18 additions & 6 deletions conversion.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,17 @@ type BatchNewLeafNodeData struct {

// BatchNewLeafNode creates a new leaf node from the given data. It optimizes LeafNode creation
// by batching expensive cryptography operations. It returns the LeafNodes sorted by stem.
func BatchNewLeafNode(nodesValues []BatchNewLeafNodeData) []LeafNode {
func BatchNewLeafNode(nodesValues []BatchNewLeafNodeData) ([]LeafNode, error) {
cfg := GetConfig()
ret := make([]LeafNode, len(nodesValues))

numBatches := runtime.NumCPU()
batchSize := len(nodesValues) / numBatches

var wg sync.WaitGroup
var (
wg sync.WaitGroup
err error
Copy link
Collaborator

@jsign jsign Jun 16, 2023

Choose a reason for hiding this comment

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

This err is accessed by parallel goroutines so it will be a race condition.

I've created a fix for this using errgroup in this PR, you might want to take a look. It's targeting this PR.

Edit: I see that test failed for this reason.

)
wg.Add(numBatches)
for i := 0; i < numBatches; i++ {
start := i * batchSize
Expand All @@ -42,7 +45,12 @@ func BatchNewLeafNode(nodesValues []BatchNewLeafNodeData) []LeafNode {
valsslice[idx] = nv.Values[idx]
}

ret[i] = *NewLeafNode(nv.Stem, valsslice)
var leaf *LeafNode
leaf, err = NewLeafNode(nv.Stem, valsslice)
if err != nil {
return
}
ret[i] = *leaf

c1c2points[2*i], c1c2points[2*i+1] = ret[i].c1, ret[i].c2
c1c2frs[2*i], c1c2frs[2*i+1] = new(Fr), new(Fr)
Expand All @@ -53,7 +61,9 @@ func BatchNewLeafNode(nodesValues []BatchNewLeafNodeData) []LeafNode {
var poly [NodeWidth]Fr
poly[0].SetUint64(1)
for i, nv := range nodesValues {
StemFromBytes(&poly[1], nv.Stem)
if err = StemFromBytes(&poly[1], nv.Stem); err != nil {
return
}
poly[2] = *c1c2frs[2*i]
poly[3] = *c1c2frs[2*i+1]

Expand All @@ -68,7 +78,7 @@ func BatchNewLeafNode(nodesValues []BatchNewLeafNodeData) []LeafNode {
return bytes.Compare(ret[i].stem, ret[j].stem) < 0
})

return ret
return ret, nil
}

// firstDiffByteIdx will return the first index in which the two stems differ.
Expand Down Expand Up @@ -127,7 +137,9 @@ func (n *InternalNode) InsertMigratedLeaves(leaves []LeafNode, resolver NodeReso
}
}

node.updateMultipleLeaves(nonPresentValues)
if err := node.updateMultipleLeaves(nonPresentValues); err != nil {
return nil
gballet marked this conversation as resolved.
Show resolved Hide resolved
}
continue
}

Expand Down
10 changes: 8 additions & 2 deletions encoding_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,10 @@ func TestLeafStemLength(t *testing.T) {
// Serialize a leaf with no values, but whose stem is 32 bytes. The
// serialization should trim the extra byte.
toolong := make([]byte, 32)
leaf := NewLeafNode(toolong, make([][]byte, NodeWidth))
leaf, err := NewLeafNode(toolong, make([][]byte, NodeWidth))
if err != nil {
t.Fatal(err)
}
ser, err := leaf.Serialize()
if err != nil {
t.Fatal(err)
Expand All @@ -32,7 +35,10 @@ func TestInvalidNodeEncoding(t *testing.T) {
// Test an invalid node type.
values := make([][]byte, NodeWidth)
values[42] = testValue
ln := NewLeafNode(ffx32KeyTest, values)
ln, err := NewLeafNode(ffx32KeyTest, values)
if err != nil {
t.Fatal(err)
}
lnbytes, err := ln.Serialize()
if err != nil {
t.Fatalf("serializing leaf node: %v", err)
Expand Down
13 changes: 8 additions & 5 deletions ipa.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@
package verkle

import (
"errors"

"github.com/crate-crypto/go-ipa/bandersnatch/fr"
"github.com/crate-crypto/go-ipa/banderwagon"
)
Expand Down Expand Up @@ -57,20 +59,21 @@ func toFrMultiple(res []*Fr, ps []*Point) {
banderwagon.MultiMapToScalarField(res, ps)
}

func FromLEBytes(fr *Fr, data []byte) {
func FromLEBytes(fr *Fr, data []byte) error {
if len(data) > 32 {
panic("data is too long")
return errors.New("data is too long")
}
var aligned [32]byte
copy(aligned[:], data)
fr.SetBytesLE(aligned[:])
return nil
}

func StemFromBytes(fr *Fr, data []byte) {
func StemFromBytes(fr *Fr, data []byte) error {
if len(data) != StemSize {
panic("data length must be StemSize")
return errors.New("data length must be StemSize")
}
FromLEBytes(fr, data)
return FromLEBytes(fr, data)
}

func FromBytes(fr *Fr, data []byte) {
Expand Down
Loading