Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
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
5 changes: 3 additions & 2 deletions cmd/proofgen/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -402,9 +402,9 @@ func staticConsistencyProbes(dir string) error {
tests := []consistencyProbe{
{0, 0, root1, root2, proof1, "sizes are equal (zero) but roots are not", true},
{1, 1, root1, root2, proof1, "sizes are equal (one) but roots are not", true},
// Sizes that are always consistent.
{0, 0, root1, root1, proof1, "sizes are equal (zero) and proof is empty", false},
{0, 0, root1, root1, proof1, "sizes are equal (zero) and proof is empty", true},
{0, 1, root1, root2, proof1, "size1 is zero and does not equal size2", true},
// Sizes that are always consistent.
{1, 1, root2, root2, proof1, "sizes are equal (one) and proof is empty", false},
// Time travel to the past.
{1, 0, root1, root2, proof1, "size1 is greater than size2", true},
Expand All @@ -418,6 +418,7 @@ func staticConsistencyProbes(dir string) error {
{0, 0, sha256EmptyTreeHash, sha256EmptyTreeHash, proof2, "sizes match but proof is not empty and sizes are zero", true},
{1, 1, sha256EmptyTreeHash, sha256EmptyTreeHash, proof2, "sizes match but proof is not empty and sizes are one", true},
// Fail to validate empty tree
{0, 1, sha256EmptyTreeHash, root2, proof1, "size1 is zero and size2 is not zero", true},
{0, 1, sha256EmptyTreeHash, sha256EmptyTreeHash, proof2, "consistency check on empty tree (size1 is zero) is useless", true},
}

Expand Down
7 changes: 5 additions & 2 deletions proof/proof.go
Original file line number Diff line number Diff line change
Expand Up @@ -82,12 +82,15 @@ func SubtreeInclusion(index, start, end uint64) (Nodes, error) {

// Consistency returns the information on how to fetch and construct a
// consistency proof between the two given tree sizes of a log Merkle tree. It
// requires 0 <= size1 <= size2.
// requires 0 < size1 <= size2.
func Consistency(size1, size2 uint64) (Nodes, error) {
if size1 > size2 {
return Nodes{}, fmt.Errorf("tree size %d > %d", size1, size2)
}
if size1 == size2 || size1 == 0 {
if size1 == 0 {
return Nodes{}, fmt.Errorf("consistency proof from empty tree is meaningless")
}
if size1 == size2 {
return Nodes{IDs: []compact.NodeID{}}, nil
}

Expand Down
2 changes: 2 additions & 0 deletions proof/proof_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -341,6 +341,8 @@ func TestConsistency(t *testing.T) {
// Errors.
{size1: 5, size2: 0, wantErr: true},
{size1: 9, size2: 8, wantErr: true},
{size1: 0, size2: 5, wantErr: true},
{size1: 0, size2: 0, wantErr: true},

{size1: 1, size2: 2, want: nodes(id(0, 1))}, // b
{size1: 1, size2: 4, want: nodes(id(0, 1), id(1, 1))}, // b h
Expand Down
4 changes: 2 additions & 2 deletions proof/verify.go
Original file line number Diff line number Diff line change
Expand Up @@ -112,13 +112,13 @@ func RootFromConsistencyProof(hasher merkle.LogHasher, size1, size2 uint64, proo
switch {
case size2 < size1:
return nil, fmt.Errorf("size2 (%d) < size1 (%d)", size1, size2)
case size1 == 0:
return nil, errors.New("consistency proof from empty tree is meaningless")
Comment on lines +115 to +116

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

For consistency with the error message in proof/proof.go (which uses "consistency proof from an empty tree is meaningless"), please update this error message to include the article "an".

Suggested change
case size1 == 0:
return nil, errors.New("consistency proof from empty tree is meaningless")
case size1 == 0:
return nil, errors.New("consistency proof from an empty tree is meaningless")

case size1 == size2:
if len(proof) > 0 {
return nil, errors.New("size1=size2, but proof is not empty")
}
return root1, nil
case size1 == 0:
return nil, errors.New("consistency proof from empty tree is meaningless")
case len(proof) == 0:
return nil, errors.New("empty proof")
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
{
"size1": 0,
"size2": 1,
"root1": "47DEQpj8HBSa+/TImW+5JCeuQeRkm5NMpJWZG3hSuFU=",
"root2": "ZG9uJ3QgY2FyZSAy",
"proof": [],
"desc": "size1 is zero and size2 is not zero",
"wantErr": true
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,5 +5,5 @@
"root2": "ZG9uJ3QgY2FyZSAx",
"proof": [],
"desc": "sizes are equal (zero) and proof is empty",
"wantErr": false
"wantErr": true
}
5 changes: 4 additions & 1 deletion testonly/tree_fuzz_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ func FuzzConsistencyProofAndVerify(f *testing.F) {
if begin > end || end > size {
return
}
if begin == 0 && end > 0 {
if begin == 0 {
return
}
tree := newTree(genEntries(size))
Expand Down Expand Up @@ -176,6 +176,9 @@ func FuzzConsistencyProofAgainstReferenceImplementation(f *testing.F) {
if begin > end || end > size {
return
}
if begin == 0 {
return
}
entries := genEntries(size)
tree := newTree(entries)
got, err := tree.ConsistencyProof(begin, end)
Expand Down
9 changes: 6 additions & 3 deletions testonly/tree_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -154,8 +154,11 @@ func TestTreeConsistencyProof(t *testing.T) {
if _, err := mt.ConsistencyProof(6, 3); err == nil {
t.Error("ConsistencyProof(6, 3) succeeded unexpectedly")
}
if _, err := mt.ConsistencyProof(0, 3); err == nil {
t.Error("ConsistencyProof(0, 3) succeeded unexpectedly")
}

for size1 := range uint64(8) + 1 {
for size1 := uint64(1); size1 <= 8; size1++ {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

high

In TestTreeConsistencyProofFuzz (around line 184), size2 and size1 are generated using rand.Uint64N(treeSize + 1) and rand.Uint64N(size2 + 1). This allows size1 to be 0. Since consistency proofs from a 0-sized tree are now considered invalid and return an error, this will cause TestTreeConsistencyProofFuzz to fail when size1 is randomly chosen as 0. To fix this, we should ensure size1 and size2 are at least 1 by adjusting the random generation to:\n\ngo\nsize2 := rand.Uint64N(treeSize) + 1\nsize1 := rand.Uint64N(size2) + 1\n

for size2 := size1; size2 <= 8; size2++ {
t.Run(fmt.Sprintf("%d:%d", size1, size2), func(t *testing.T) {
got, err := mt.ConsistencyProof(size1, size2)
Expand All @@ -178,8 +181,8 @@ func TestTreeConsistencyProofFuzz(t *testing.T) {
for treeSize := uint64(1); treeSize <= 256; treeSize++ {
mt := newTree(entries[:treeSize])
for range 8 {
size2 := rand.Uint64N(treeSize + 1)
size1 := rand.Uint64N(size2 + 1)
size2 := rand.Uint64N(treeSize) + 1
size1 := rand.Uint64N(size2) + 1

got, err := mt.ConsistencyProof(size1, size2)
if err != nil {
Expand Down
Loading