From f403fd983b1c494d74b4b6e3e5db1d4a344324ac Mon Sep 17 00:00:00 2001 From: Sanaz Taheri <35961250+staheri14@users.noreply.github.com> Date: Mon, 1 May 2023 08:26:53 -0700 Subject: [PATCH] feat!: handles empty proofs in validateLeafHash and VerifyInclusion (#184) ## 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 --- proof.go | 23 ++++++++++++-- proof_test.go | 88 ++++++++++++++++++++++++++++++++++++++++++++++++--- 2 files changed, 104 insertions(+), 7 deletions(-) diff --git a/proof.go b/proof.go index 3e854af7..01034a91 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: diff --git a/proof_test.go b/proof_test.go index c48c36b9..1178bf52 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}