Repository: celestiaorg/nmt
Pull request #184 (2023-04-26 23:13:37)

feat!: handles empty proofs in validateLeafHash and VerifyInclusion
## Overview
Closes https://github.com/celestiaorg/nmt/issues/140
The changes made in this PR are considered to be breaking, as they modify the behavior of `VerifyInclusion`. In the prior version, the `VerifyInclusion` function would return `false` for the first test case named ["valid empty proof and leaves == empty."](https://github.com/celestiaorg/nmt/blob/712ed08a521b254b62a707a3f2d041d3f3a8d83f/proof_test.go#L442) of the `TestVerifyInclusion_EmptyProofs` suite. However, with the changes implemented, an empty proof for an empty set of leaves is now considered a valid proof. As a result, `VerifyInclusion` now returns `true`.
cc: @liamsi 

## Checklist

- [x] New and updated code has appropriate documentation
- [x] New and updated code has new and/or updated testing
- [x] Required CI checks are passing
- [x] Visual proof for any user facing features like CLI or documentation updates
- [x] Linked issues closed with keywords



Patch:
diff --git a/proof.go b/proof.go
index 3e854af..01034a9 100644
--- a/proof.go
+++ b/proof.go
@@ -229,6 +229,11 @@ func (proof Proof) VerifyNamespace(h hash.Hash, nID namespace.ID, leaves [][]byt
 // tree represented by the root parameter that matches the namespace ID nID
 // but is not present in the leafHashes list.
 func (proof Proof) verifyLeafHashes(nth *Hasher, verifyCompleteness bool, nID namespace.ID, leafHashes [][]byte, root []byte) (bool, error) {
+	// check that the proof range is valid
+	if proof.Start() < 0 || proof.Start() >= proof.End() {
+		return false, fmt.Errorf("proof range [proof.start=%d, proof.end=%d) is not valid: %w", proof.Start(), proof.End(), ErrInvalidRange)
+	}
+
 	// perform some consistency checks:
 	if nID.Size() != nth.NamespaceSize() {
 		return false, fmt.Errorf("namespace ID size (%d) does not match the namespace size of the NMT hasher (%d)", nID.Size(), nth.NamespaceSize())
@@ -287,7 +292,7 @@ func (proof Proof) verifyLeafHashes(nth *Hasher, verifyCompleteness bool, nID na
 		if end-start == 1 {
 			// if the leaf index falls within the proof range, pop and return a
 			// leaf
-			if proof.start <= start && start < proof.end {
+			if proof.Start() <= start && start < proof.End() {
 				leafHash := leafHashes[0]
 				// advance leafHashes
 				leafHashes = leafHashes[1:]
@@ -303,7 +308,7 @@ func (proof Proof) verifyLeafHashes(nth *Hasher, verifyCompleteness bool, nID na
 		// if current range does not overlap with the proof range, pop and
 		// return a proof node if present, else return nil because subtree
 		// doesn't exist
-		if end <= proof.start || start >= proof.end {
+		if end <= proof.Start() || start >= proof.End() {
 			return popIfNonEmpty(&proof.nodes), nil
 		}
 
@@ -354,6 +359,20 @@ func (proof Proof) verifyLeafHashes(nth *Hasher, verifyCompleteness bool, nID na
 // `nid`.
 // VerifyInclusion does not verify the completeness of the proof, so it's possible for leavesWithoutNamespace to be a subset of the leaves in the tree that have the namespace ID nid.
 func (proof Proof) VerifyInclusion(h hash.Hash, nid namespace.ID, leavesWithoutNamespace [][]byte, root []byte) bool {
+	// check the range of the proof
+	isEmptyRange := proof.start == proof.end
+	if isEmptyRange {
+		// the only case in which an empty proof is valid is when the supplied leavesWithoutNamespace is also empty.
+		// rationale: no proof (i.e., an empty proof) is needed to prove that an empty set of leaves belong to the tree with root `root`.
+		// unlike VerifyNamespace(), we do not care about the queried `nid` here, because  VerifyInclusion does not verify the completeness of the proof
+		// i.e., whether the leavesWithoutNamespace is the full set of leaves matching the queried `nid`.
+		if proof.IsEmptyProof() && len(leavesWithoutNamespace) == 0 {
+			return true
+		}
+		// if the proof range is empty but !proof.IsEmptyProof() || len(leavesWithoutNamespace) != 0, then the verification should fail
+		return false
+	}
+
 	nth := NewNmtHasher(h, nid.Size(), proof.isMaxNamespaceIDIgnored)
 
 	// perform some consistency checks:


Test patch:
diff --git a/proof_test.go b/proof_test.go
index c48c36b..1178bf5 100644
--- a/proof_test.go
+++ b/proof_test.go
@@ -280,7 +280,8 @@ func safeAppend(id, data []byte) []byte {
 
 func TestVerifyLeafHashes_Err(t *testing.T) {
 	// create a sample tree
-	nmt := exampleNMT(2, 1, 2, 3, 4, 5, 6, 7, 8)
+	nameIDSize := 2
+	nmt := exampleNMT(nameIDSize, 1, 2, 3, 4, 5, 6, 7, 8)
 	hasher := nmt.treeHasher
 	root, err := nmt.Root()
 	require.NoError(t, err)
@@ -293,16 +294,41 @@ func TestVerifyLeafHashes_Err(t *testing.T) {
 	// note that the leaf at index 4 has the namespace ID of 5.
 	leafHash5 := nmt.leafHashes[4][:nmt.NamespaceSize()]
 
+	// corrupt the leafHash: replace its namespace ID with a different one.
+	nID3 := createByteSlice(nameIDSize, 3)
+	leafHash5SmallerNID := concat(nID3, nID3, nmt.leafHashes[4][2*nmt.NamespaceSize():])
+	require.NoError(t, hasher.ValidateNodeFormat(leafHash5SmallerNID))
+
+	nID6 := createByteSlice(nameIDSize, 7)
+	leafHash5BiggerNID := concat(nID6, nID6, nmt.leafHashes[4][2*nmt.NamespaceSize():])
+	require.NoError(t, hasher.ValidateNodeFormat(leafHash5BiggerNID))
+
 	// create nmt proof for namespace ID 4
 	nID4 := namespace.ID{4, 4}
-	proof4, err := nmt.ProveNamespace(nID4)
+	proof4InvalidNodes, err := nmt.ProveNamespace(nID4)
 	require.NoError(t, err)
 	// corrupt the last node in the proof4.nodes, it resides on the right side of the proof4.end index.
 	// this test scenario makes the proof verification fail when constructing the tree root from the
 	// computed subtree root and the proof.nodes on the right side of the proof.end index.
-	proof4.nodes[2] = proof4.nodes[2][:nmt.NamespaceSize()-1]
+	proof4InvalidNodes.nodes[2] = proof4InvalidNodes.nodes[2][:nmt.NamespaceSize()-1]
 	leafHash4 := nmt.leafHashes[3]
 
+	// create a proof with invalid range: start = end = 0
+	proof4InvalidRangeSEE, err := nmt.ProveNamespace(nID4)
+	require.NoError(t, err)
+	proof4InvalidRangeSEE.end = 0
+	proof4InvalidRangeSEE.start = 0
+
+	// create a proof with invalid range: start > end
+	proof4InvalidRangeSBE, err := nmt.ProveNamespace(nID4)
+	require.NoError(t, err)
+	proof4InvalidRangeSBE.start = proof4InvalidRangeSBE.end + 1
+
+	// create a proof with invalid range: start < 0
+	proof4InvalidRangeSLZ, err := nmt.ProveNamespace(nID4)
+	require.NoError(t, err)
+	proof4InvalidRangeSLZ.start = -1
+
 	tests := []struct {
 		name               string
 		proof              Proof
@@ -314,9 +340,13 @@ func TestVerifyLeafHashes_Err(t *testing.T) {
 		wantErr            bool
 	}{
 		{"wrong leafHash: not namespaced", proof5, hasher, true, nID5, [][]byte{leafHash5}, root, true},
-		{"wrong leafHash: incorrect namespace", proof5, hasher, true, nID5, [][]byte{{10, 10, 10, 10}}, root, true},
-		{"wrong proof.nodes: the last node has an incorrect format", proof4, hasher, false, nID4, [][]byte{leafHash4}, root, true},
+		{"wrong leafHash: smaller namespace", proof5, hasher, true, nID5, [][]byte{leafHash5SmallerNID}, root, true},
+		{"wong leafHash: bigger namespace", proof5, hasher, true, nID5, [][]byte{leafHash5BiggerNID}, root, true},
+		{"wrong proof.nodes: the last node has an incorrect format", proof4InvalidNodes, hasher, false, nID4, [][]byte{leafHash4}, root, true},
 		//  the verifyCompleteness parameter in the verifyProof function should be set to false in order to bypass nodes correctness check during the completeness verification (otherwise it panics).
+		{"wrong proof range: start = end", proof4InvalidRangeSEE, hasher, true, nID4, [][]byte{leafHash4}, root, true},
+		{"wrong proof range: start > end", proof4InvalidRangeSBE, hasher, true, nID4, [][]byte{leafHash4}, root, true},
+		{"wrong proof range: start < 0", proof4InvalidRangeSLZ, hasher, true, nID4, [][]byte{leafHash4}, root, true},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
@@ -377,6 +407,54 @@ func TestVerifyInclusion_False(t *testing.T) {
 	}
 }
 
+// TestVerifyInclusion_EmptyProofs tests the correct behaviour of VerifyInclusion in response to valid and invalid empty proofs.
+func TestVerifyInclusion_EmptyProofs(t *testing.T) {
+	hasher := sha256.New()
+
+	// create a tree
+	nIDSize := 1
+	tree := exampleNMT(nIDSize, 1, 2, 3, 4, 5, 6, 7, 8)
+	root, err := tree.Root()
+	require.NoError(t, err)
+
+	sampleLeafWithoutNID := tree.leaves[3][tree.NamespaceSize():] // does not matter which leaf we choose, just a leaf that belongs to the tree
+	sampleNID := tree.leaves[3][:tree.NamespaceSize()]            // the NID of the leaf we chose
+	sampleNode := tree.leafHashes[7]                              // does not matter which node we choose, just a node that belongs to the tree
+
+	// create an empty proof
+	emptyProof := Proof{}
+	// verify that the proof is a valid empty proof
+	// this check is to ensure that we stay consistent with the definition of empty proofs
+	require.True(t, emptyProof.IsEmptyProof())
+
+	// create a non-empty proof
+	nonEmptyProof := Proof{nodes: [][]byte{sampleNode}}
+
+	type args struct {
+		hasher                 hash.Hash
+		nID                    namespace.ID
+		leavesWithoutNamespace [][]byte
+		root                   []byte
+	}
+	tests := []struct {
+		name   string
+		proof  Proof
+		args   args
+		result bool
+	}{
+		{"valid empty proof and leaves == empty", emptyProof, args{hasher, sampleNID, [][]byte{}, root}, true},
+		{"valid empty proof and leaves == non-empty", emptyProof, args{hasher, sampleNID, [][]byte{sampleLeafWithoutNID}, root}, false},
+		{"invalid empty proof and leaves == empty", nonEmptyProof, args{hasher, sampleNID, [][]byte{}, root}, false},
+		{"invalid empty proof and leaves != empty", nonEmptyProof, args{hasher, sampleNID, [][]byte{sampleLeafWithoutNID}, root}, false},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got := tt.proof.VerifyInclusion(tt.args.hasher, tt.args.nID, tt.args.leavesWithoutNamespace, tt.args.root)
+			assert.Equal(t, tt.result, got)
+		})
+	}
+}
+
 func TestVerifyNamespace_False(t *testing.T) {
 	nIDs := []byte{1, 2, 3, 4, 5, 6, 7, 8, 11}