From e49d7b104ea11e791ab4d7c0434155b7b9d2209a Mon Sep 17 00:00:00 2001 From: sanaz Date: Fri, 21 Apr 2023 16:16:38 -0700 Subject: [PATCH 01/34] updates criteria of an empty proof --- proof.go | 8 ++++++-- proof_test.go | 49 +++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 55 insertions(+), 2 deletions(-) diff --git a/proof.go b/proof.go index 32d8480..7ba9987 100644 --- a/proof.go +++ b/proof.go @@ -106,6 +106,11 @@ func NewAbsenceProof(proofStart, proofEnd int, proofNodes [][]byte, leafHash []b return Proof{proofStart, proofEnd, proofNodes, leafHash, ignoreMaxNamespace} } +// isEmptyProof checks whether the proof corresponds to an empty proof. +func (proof Proof) isEmptyProof() bool { + return proof.start == proof.end +} + // VerifyNamespace verifies a whole namespace, i.e. 1) it verifies inclusion of // the provided `data` in the tree (or the proof.leafHash in case of absence // proof) 2) it verifies that the namespace is complete i.e., the data items @@ -149,8 +154,7 @@ func (proof Proof) VerifyNamespace(h hash.Hash, nID namespace.ID, leaves [][]byt } } - isEmptyRange := proof.start == proof.end - if len(leaves) == 0 && isEmptyRange && len(proof.nodes) == 0 { + if proof.isEmptyProof() { // empty proofs are always rejected unless nID is outside the range of // namespaces covered by the root we special case the empty root, since // it purports to cover the zero namespace but does not actually include diff --git a/proof_test.go b/proof_test.go index 6cd3f0e..b69eba0 100644 --- a/proof_test.go +++ b/proof_test.go @@ -13,6 +13,55 @@ import ( "github.com/celestiaorg/nmt/namespace" ) +// TestVerifyNamespace_EmptyProof tests the correct behaviour of VerifyNamespace for valid and invalid empty proofs. +func TestVerifyNamespace_EmptyProof(t *testing.T) { + + // create a tree with 4 leaves + nIDSize := 1 + tree := exampleNMT(nIDSize, 1, 2, 3, 4) + root, err := tree.Root() + require.NoError(t, err) + + // build a proof for an NID that is outside tree range of the tree + nID0 := []byte{0} + validEmptyProof, err := tree.ProveNamespace(nID0) + require.NoError(t, err) + data0 := [][]byte{} + + // build a proof for an NID that is within the namespace range of the tree + nID1 := []byte{1} + invalidEmptyProof, err := tree.ProveNamespace([]byte{1}) + require.NoError(t, err) + data1 := [][]byte{tree.leaves[0]} + // modify the proof to be empty + invalidEmptyProof.start = invalidEmptyProof.end + + hasher := sha256.New() + type args struct { + proof Proof + hasher hash.Hash + nID namespace.ID + leaves [][]byte + root []byte + } + + tests := []struct { + name string + args args + want bool + }{ + {"valid empty proof", args{validEmptyProof, hasher, nID0, data0, root}, true}, + {"invalid empty proof", args{invalidEmptyProof, hasher, nID1, data1, root}, false}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := tt.args.proof.VerifyNamespace(tt.args.hasher, tt.args.nID, tt.args.leaves, tt.args.root); got != tt.want { + t.Errorf("VerifyNamespace() = %v, want %v", got, tt.want) + } + }) + } +} + func TestProof_VerifyNamespace_False(t *testing.T) { const testNidLen = 3 From 5e9091c5785b8a481c45f05c28931af655a697a1 Mon Sep 17 00:00:00 2001 From: sanaz Date: Fri, 21 Apr 2023 16:17:44 -0700 Subject: [PATCH 02/34] replaces slice literal with nID1 --- proof_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/proof_test.go b/proof_test.go index b69eba0..b0d3f5e 100644 --- a/proof_test.go +++ b/proof_test.go @@ -30,7 +30,7 @@ func TestVerifyNamespace_EmptyProof(t *testing.T) { // build a proof for an NID that is within the namespace range of the tree nID1 := []byte{1} - invalidEmptyProof, err := tree.ProveNamespace([]byte{1}) + invalidEmptyProof, err := tree.ProveNamespace(nID1) require.NoError(t, err) data1 := [][]byte{tree.leaves[0]} // modify the proof to be empty From b6e3cd30bec87bec87031cfed9e093d3604d0fc6 Mon Sep 17 00:00:00 2001 From: sanaz Date: Mon, 24 Apr 2023 15:50:23 -0700 Subject: [PATCH 03/34] refactors the code to reject invalid empty proof, adds tests --- proof.go | 39 ++++++++++++++++++++++++--------------- proof_test.go | 44 +++++++++++++++++++++++++++++++++----------- 2 files changed, 57 insertions(+), 26 deletions(-) diff --git a/proof.go b/proof.go index 7ba9987..7cfc574 100644 --- a/proof.go +++ b/proof.go @@ -4,10 +4,9 @@ import ( "bytes" "errors" "fmt" + "github.com/celestiaorg/nmt/namespace" "hash" "math/bits" - - "github.com/celestiaorg/nmt/namespace" ) // ErrFailedCompletenessCheck indicates that the verification of a namespace proof failed due to the lack of completeness property. @@ -106,9 +105,9 @@ func NewAbsenceProof(proofStart, proofEnd int, proofNodes [][]byte, leafHash []b return Proof{proofStart, proofEnd, proofNodes, leafHash, ignoreMaxNamespace} } -// isEmptyProof checks whether the proof corresponds to an empty proof. -func (proof Proof) isEmptyProof() bool { - return proof.start == proof.end +// IsOfEmptyProof checks whether the proof corresponds to an out of range proof. +func (proof Proof) IsOfEmptyProof() bool { + return proof.start == proof.end && len(proof.nodes) == 0 } // VerifyNamespace verifies a whole namespace, i.e. 1) it verifies inclusion of @@ -154,18 +153,28 @@ func (proof Proof) VerifyNamespace(h hash.Hash, nID namespace.ID, leaves [][]byt } } - if proof.isEmptyProof() { - // empty proofs are always rejected unless nID is outside the range of - // namespaces covered by the root we special case the empty root, since - // it purports to cover the zero namespace but does not actually include - // any such nodes - min := namespace.ID(MinNamespace(root, nIDLen)) - max := namespace.ID(MaxNamespace(root, nIDLen)) - if nID.Less(min) || max.Less(nID) || bytes.Equal(root, nth.EmptyRoot()) { - return true + isEmptyRange := proof.start == proof.end + if isEmptyRange { + if proof.IsOfEmptyProof() && len(leaves) == 0 { + min := namespace.ID(MinNamespace(root, nIDLen)) + max := namespace.ID(MaxNamespace(root, nIDLen)) + // empty proofs are always rejected unless nID is outside the range of + // namespaces covered by the root we special case the empty root, since + // it purports to cover the zero namespace but does not actually include + // any such nodes + if nID.Less(min) || max.Less(nID) { + return true + } + if bytes.Equal(root, nth.EmptyRoot()) { + return true + } + return false + } else { + // the proof range is empty, and invalid + return false } - return false } + gotLeafHashes := make([][]byte, 0, len(leaves)) if proof.IsOfAbsence() { gotLeafHashes = append(gotLeafHashes, proof.leafHash) diff --git a/proof_test.go b/proof_test.go index b0d3f5e..9f3c1f5 100644 --- a/proof_test.go +++ b/proof_test.go @@ -23,18 +23,34 @@ func TestVerifyNamespace_EmptyProof(t *testing.T) { require.NoError(t, err) // build a proof for an NID that is outside tree range of the tree + // start = end = 0, nodes = empty nID0 := []byte{0} - validEmptyProof, err := tree.ProveNamespace(nID0) + validEmptyProofZeroRange, err := tree.ProveNamespace(nID0) require.NoError(t, err) - data0 := [][]byte{} + + // build a proof for an NID that is outside tree range of the tree + // start = end = 1, nodes = nil + validEmptyProofNonZeroRange, err := tree.ProveNamespace(nID0) + require.NoError(t, err) + // modify the proof range to be non-zero, it should still be valid + validEmptyProofNonZeroRange.start = 1 + validEmptyProofNonZeroRange.end = 1 // build a proof for an NID that is within the namespace range of the tree + // start = end = 0, nodes = non-empty nID1 := []byte{1} - invalidEmptyProof, err := tree.ProveNamespace(nID1) + zeroRangeOnlyProof, err := tree.ProveNamespace(nID1) require.NoError(t, err) - data1 := [][]byte{tree.leaves[0]} - // modify the proof to be empty - invalidEmptyProof.start = invalidEmptyProof.end + // modify the proof to contain a zero range + zeroRangeOnlyProof.start = 0 + zeroRangeOnlyProof.end = 0 + + // build a proof for an NID that is within the namespace range of the tree + // start = 0, end = 1, nodes = empty + emptyNodesOnlyProof, err := tree.ProveNamespace(nID1) + require.NoError(t, err) + // modify the proof nodes to be empty + emptyNodesOnlyProof.nodes = [][]byte{} hasher := sha256.New() type args struct { @@ -46,15 +62,21 @@ func TestVerifyNamespace_EmptyProof(t *testing.T) { } tests := []struct { - name string - args args - want bool + name string + args args + want bool + isValidEmptyProof bool }{ - {"valid empty proof", args{validEmptyProof, hasher, nID0, data0, root}, true}, - {"invalid empty proof", args{invalidEmptyProof, hasher, nID1, data1, root}, false}, + {"valid empty proof with (start == end) == 0 and empty leaves", args{validEmptyProofZeroRange, hasher, nID0, [][]byte{}, root}, true, true}, + {"valid empty proof with (start == end) != 0 and empty leaves", args{validEmptyProofNonZeroRange, hasher, nID0, [][]byte{}, root}, true, true}, + {"valid empty proof with (start == end) == 0 and non-empty leaves", args{validEmptyProofZeroRange, hasher, nID0, [][]byte{{1}}, root}, false, true}, + {"valid empty proof with (start == end) != 0 and non-empty leaves", args{validEmptyProofNonZeroRange, hasher, nID0, [][]byte{{1}}, root}, false, true}, + {"invalid empty proof: start == end == 0, nodes == non-empty", args{zeroRangeOnlyProof, hasher, nID1, [][]byte{}, root}, false, false}, + {"invalid empty proof: start == 0, end == 1, nodes == empty", args{emptyNodesOnlyProof, hasher, nID1, [][]byte{}, root}, false, false}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { + require.True(t, tt.args.proof.IsOfEmptyProof() == tt.isValidEmptyProof) if got := tt.args.proof.VerifyNamespace(tt.args.hasher, tt.args.nID, tt.args.leaves, tt.args.root); got != tt.want { t.Errorf("VerifyNamespace() = %v, want %v", got, tt.want) } From 5c32cbc4498431c77b5cf8e4c69dfbc94f23f8e3 Mon Sep 17 00:00:00 2001 From: sanaz Date: Mon, 24 Apr 2023 15:54:14 -0700 Subject: [PATCH 04/34] corrects IsOfEmptyProof description --- proof.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/proof.go b/proof.go index 7cfc574..f473755 100644 --- a/proof.go +++ b/proof.go @@ -105,7 +105,7 @@ func NewAbsenceProof(proofStart, proofEnd int, proofNodes [][]byte, leafHash []b return Proof{proofStart, proofEnd, proofNodes, leafHash, ignoreMaxNamespace} } -// IsOfEmptyProof checks whether the proof corresponds to an out of range proof. +// IsOfEmptyProof checks whether the proof corresponds to an empty proof as defined in NMT specifications https://github.com/celestiaorg/nmt/blob/master/docs/spec/nmt.md. func (proof Proof) IsOfEmptyProof() bool { return proof.start == proof.end && len(proof.nodes) == 0 } From 84865d7b545af1e1eea6be840bf04f67750b88fa Mon Sep 17 00:00:00 2001 From: sanaz Date: Mon, 24 Apr 2023 15:59:43 -0700 Subject: [PATCH 05/34] prefixes min and max with root --- proof.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/proof.go b/proof.go index f473755..38bbf6c 100644 --- a/proof.go +++ b/proof.go @@ -156,13 +156,13 @@ func (proof Proof) VerifyNamespace(h hash.Hash, nID namespace.ID, leaves [][]byt isEmptyRange := proof.start == proof.end if isEmptyRange { if proof.IsOfEmptyProof() && len(leaves) == 0 { - min := namespace.ID(MinNamespace(root, nIDLen)) - max := namespace.ID(MaxNamespace(root, nIDLen)) - // empty proofs are always rejected unless nID is outside the range of - // namespaces covered by the root we special case the empty root, since + rootMin := namespace.ID(MinNamespace(root, nIDLen)) + rootMax := namespace.ID(MaxNamespace(root, nIDLen)) + // empty proofs are always rejected unless 1) nID is outside the range of + // namespaces covered by the root 2) the root represents an empty tree, since // it purports to cover the zero namespace but does not actually include // any such nodes - if nID.Less(min) || max.Less(nID) { + if nID.Less(rootMin) || rootMax.Less(nID) { return true } if bytes.Equal(root, nth.EmptyRoot()) { From c8a805984fe98bd7585bf113c5cb75342f92df29 Mon Sep 17 00:00:00 2001 From: sanaz Date: Mon, 24 Apr 2023 16:02:39 -0700 Subject: [PATCH 06/34] fixes linter issues --- proof.go | 8 ++++---- proof_test.go | 1 - 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/proof.go b/proof.go index 38bbf6c..773f17a 100644 --- a/proof.go +++ b/proof.go @@ -4,9 +4,10 @@ import ( "bytes" "errors" "fmt" - "github.com/celestiaorg/nmt/namespace" "hash" "math/bits" + + "github.com/celestiaorg/nmt/namespace" ) // ErrFailedCompletenessCheck indicates that the verification of a namespace proof failed due to the lack of completeness property. @@ -169,10 +170,9 @@ func (proof Proof) VerifyNamespace(h hash.Hash, nID namespace.ID, leaves [][]byt return true } return false - } else { - // the proof range is empty, and invalid - return false } + // the proof range is empty, and invalid + return false } gotLeafHashes := make([][]byte, 0, len(leaves)) diff --git a/proof_test.go b/proof_test.go index 9f3c1f5..6864dbc 100644 --- a/proof_test.go +++ b/proof_test.go @@ -15,7 +15,6 @@ import ( // TestVerifyNamespace_EmptyProof tests the correct behaviour of VerifyNamespace for valid and invalid empty proofs. func TestVerifyNamespace_EmptyProof(t *testing.T) { - // create a tree with 4 leaves nIDSize := 1 tree := exampleNMT(nIDSize, 1, 2, 3, 4) From 8a276366b706a678c296a10aa718fc1d0667fb14 Mon Sep 17 00:00:00 2001 From: sanaz Date: Tue, 25 Apr 2023 09:06:18 -0700 Subject: [PATCH 07/34] revises some typos --- proof_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/proof_test.go b/proof_test.go index 6864dbc..29672ff 100644 --- a/proof_test.go +++ b/proof_test.go @@ -21,13 +21,13 @@ func TestVerifyNamespace_EmptyProof(t *testing.T) { root, err := tree.Root() require.NoError(t, err) - // build a proof for an NID that is outside tree range of the tree + // build a proof for an NID that is outside the namespace range of the tree // start = end = 0, nodes = empty nID0 := []byte{0} validEmptyProofZeroRange, err := tree.ProveNamespace(nID0) require.NoError(t, err) - // build a proof for an NID that is outside tree range of the tree + // build a proof for an NID that is outside the namespace range of the tree // start = end = 1, nodes = nil validEmptyProofNonZeroRange, err := tree.ProveNamespace(nID0) require.NoError(t, err) From 9e3d8de24adb44c98c3ca644b56da63e1f38a715 Mon Sep 17 00:00:00 2001 From: sanaz Date: Tue, 25 Apr 2023 13:37:45 -0700 Subject: [PATCH 08/34] renames IsOfEmptyProof to IsEmptyProof --- proof.go | 6 +++--- proof_test.go | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/proof.go b/proof.go index 773f17a..3e854af 100644 --- a/proof.go +++ b/proof.go @@ -106,8 +106,8 @@ func NewAbsenceProof(proofStart, proofEnd int, proofNodes [][]byte, leafHash []b return Proof{proofStart, proofEnd, proofNodes, leafHash, ignoreMaxNamespace} } -// IsOfEmptyProof checks whether the proof corresponds to an empty proof as defined in NMT specifications https://github.com/celestiaorg/nmt/blob/master/docs/spec/nmt.md. -func (proof Proof) IsOfEmptyProof() bool { +// IsEmptyProof checks whether the proof corresponds to an empty proof as defined in NMT specifications https://github.com/celestiaorg/nmt/blob/master/docs/spec/nmt.md. +func (proof Proof) IsEmptyProof() bool { return proof.start == proof.end && len(proof.nodes) == 0 } @@ -156,7 +156,7 @@ func (proof Proof) VerifyNamespace(h hash.Hash, nID namespace.ID, leaves [][]byt isEmptyRange := proof.start == proof.end if isEmptyRange { - if proof.IsOfEmptyProof() && len(leaves) == 0 { + if proof.IsEmptyProof() && len(leaves) == 0 { rootMin := namespace.ID(MinNamespace(root, nIDLen)) rootMax := namespace.ID(MaxNamespace(root, nIDLen)) // empty proofs are always rejected unless 1) nID is outside the range of diff --git a/proof_test.go b/proof_test.go index 29672ff..c48c36b 100644 --- a/proof_test.go +++ b/proof_test.go @@ -75,7 +75,7 @@ func TestVerifyNamespace_EmptyProof(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - require.True(t, tt.args.proof.IsOfEmptyProof() == tt.isValidEmptyProof) + require.True(t, tt.args.proof.IsEmptyProof() == tt.isValidEmptyProof) if got := tt.args.proof.VerifyNamespace(tt.args.hasher, tt.args.nID, tt.args.leaves, tt.args.root); got != tt.want { t.Errorf("VerifyNamespace() = %v, want %v", got, tt.want) } From 20b2205d764bfcc952e8a52219bbe4699b790e09 Mon Sep 17 00:00:00 2001 From: sanaz Date: Tue, 25 Apr 2023 14:12:48 -0700 Subject: [PATCH 09/34] returns immediately on invalid range --- nmt.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/nmt.go b/nmt.go index 5e5ce8d..dfaec65 100644 --- a/nmt.go +++ b/nmt.go @@ -253,6 +253,11 @@ func (n *NamespacedMerkleTree) buildRangeProof(proofStart, proofEnd int) ([][]by proof := [][]byte{} // it is the list of nodes hashes (as byte slices) with no index var recurse func(start, end int, includeNode bool) ([]byte, error) + // validate the range + if proofStart < 0 || proofStart > proofEnd || proofEnd > len(n.leafHashes) { + return nil, ErrInvalidRange + } + // start, end are indices of leaves in the tree hence they should be within // the size of the tree i.e., less than or equal to the len(n.leaves) // includeNode indicates whether the hash of the current subtree (covering From 6222d5f4294b42f4e16cb38001e72bd85b406358 Mon Sep 17 00:00:00 2001 From: sanaz Date: Tue, 25 Apr 2023 14:12:57 -0700 Subject: [PATCH 10/34] adds unit tests --- nmt_test.go | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/nmt_test.go b/nmt_test.go index 8b677d9..79142aa 100644 --- a/nmt_test.go +++ b/nmt_test.go @@ -863,17 +863,22 @@ func swap(slice [][]byte, i int, j int) { // Test_buildRangeProof_Err tests that buildRangeProof returns an error when the underlying tree has an invalid state e.g., leaves are not ordered by namespace ID or a leaf hash is corrupted. func Test_buildRangeProof_Err(t *testing.T) { + nIDList := []byte{1, 2, 3, 4, 5, 6, 7, 8} + nIDSize := 2 + // create a nmt, 8 leaves namespaced sequentially from 1-8 - treeWithCorruptLeafHash := exampleNMT(2, 1, 2, 3, 4, 5, 6, 7, 8) + treeWithCorruptLeafHash := exampleNMT(nIDSize, nIDList...) // corrupt a leaf hash treeWithCorruptLeafHash.leafHashes[4] = treeWithCorruptLeafHash.leafHashes[4][:treeWithCorruptLeafHash.NamespaceSize()] // create an NMT with 8 sequentially namespaced leaves, numbered from 1 to 8. - treeWithUnorderedLeafHashes := exampleNMT(2, 1, 2, 3, 4, 5, 6, 7, 8) + treeWithUnorderedLeafHashes := exampleNMT(nIDSize, nIDList...) // swap the positions of the 4th and 5th leaves swap(treeWithUnorderedLeafHashes.leaves, 4, 5) swap(treeWithUnorderedLeafHashes.leafHashes, 4, 5) + validTree := exampleNMT(nIDSize, nIDList...) + tests := []struct { name string tree *NamespacedMerkleTree @@ -887,6 +892,9 @@ func Test_buildRangeProof_Err(t *testing.T) { // not just the corrupted range. {"unordered leaf hashes: the last leaf", treeWithUnorderedLeafHashes, 7, 8, true, ErrUnorderedSiblings}, // for a tree with an unordered set of leaves, the buildRangeProof function should produce an error for any input range, // not just the corrupted range. + {"invalid proof range: start > end", validTree, 5, 4, true, ErrInvalidRange}, + {"invalid proof range: start < 0", validTree, -1, 4, true, ErrInvalidRange}, + {"invalid proof range: end > number of leaves", validTree, 0, len(validTree.leaves) + 1, true, ErrInvalidRange}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { From 9fd31f2956c4af3cff03ea44c905e1435a831858 Mon Sep 17 00:00:00 2001 From: sanaz Date: Tue, 25 Apr 2023 14:21:44 -0700 Subject: [PATCH 11/34] considers start=end as invalid --- nmt.go | 2 +- nmt_test.go | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/nmt.go b/nmt.go index dfaec65..1136bf8 100644 --- a/nmt.go +++ b/nmt.go @@ -254,7 +254,7 @@ func (n *NamespacedMerkleTree) buildRangeProof(proofStart, proofEnd int) ([][]by var recurse func(start, end int, includeNode bool) ([]byte, error) // validate the range - if proofStart < 0 || proofStart > proofEnd || proofEnd > len(n.leafHashes) { + if proofStart < 0 || proofStart >= proofEnd || proofEnd > len(n.leafHashes) { return nil, ErrInvalidRange } diff --git a/nmt_test.go b/nmt_test.go index 79142aa..0fc7677 100644 --- a/nmt_test.go +++ b/nmt_test.go @@ -893,6 +893,7 @@ func Test_buildRangeProof_Err(t *testing.T) { {"unordered leaf hashes: the last leaf", treeWithUnorderedLeafHashes, 7, 8, true, ErrUnorderedSiblings}, // for a tree with an unordered set of leaves, the buildRangeProof function should produce an error for any input range, // not just the corrupted range. {"invalid proof range: start > end", validTree, 5, 4, true, ErrInvalidRange}, + {"invalid proof range: start = end", validTree, 5, 5, true, ErrInvalidRange}, {"invalid proof range: start < 0", validTree, -1, 4, true, ErrInvalidRange}, {"invalid proof range: end > number of leaves", validTree, 0, len(validTree.leaves) + 1, true, ErrInvalidRange}, } From 167629c945ce53e5ce2f63c0b01b6a057cda90c8 Mon Sep 17 00:00:00 2001 From: sanaz Date: Tue, 25 Apr 2023 14:30:34 -0700 Subject: [PATCH 12/34] defines a utility function for proof range verification --- nmt.go | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/nmt.go b/nmt.go index 1136bf8..26c81d0 100644 --- a/nmt.go +++ b/nmt.go @@ -170,7 +170,7 @@ func (n *NamespacedMerkleTree) ProveRange(start, end int) (Proof, error) { isMaxNsIgnored := n.treeHasher.IsMaxNamespaceIDIgnored() // TODO: store nodes and re-use the hashes instead recomputing parts of the // tree here - if start < 0 || start >= end || end > len(n.leafHashes) { + if err := n.validateRange(start, end); err != nil { return NewEmptyRangeProof(isMaxNsIgnored), ErrInvalidRange } proof, err := n.buildRangeProof(start, end) @@ -245,6 +245,14 @@ func (n *NamespacedMerkleTree) ProveNamespace(nID namespace.ID) (Proof, error) { return NewAbsenceProof(proofStart, proofEnd, proof, n.leafHashes[proofStart], isMaxNsIgnored), nil } +// validateRange validates the range [start, end) against the size of the tree. +func (n *NamespacedMerkleTree) validateRange(start, end int) error { + if start < 0 || start >= end || end > len(n.leaves) { + return ErrInvalidRange + } + return nil +} + // buildRangeProof returns the nodes (as byte slices) in the range proof of the // supplied range i.e., [proofStart, proofEnd) where proofEnd is non-inclusive. // The nodes are ordered according to in order traversal of the namespaced tree. @@ -254,7 +262,7 @@ func (n *NamespacedMerkleTree) buildRangeProof(proofStart, proofEnd int) ([][]by var recurse func(start, end int, includeNode bool) ([]byte, error) // validate the range - if proofStart < 0 || proofStart >= proofEnd || proofEnd > len(n.leafHashes) { + if err := n.validateRange(proofStart, proofEnd); err != nil { return nil, ErrInvalidRange } From fcd3cd0b120447b85a002935dda0e2acc3530f93 Mon Sep 17 00:00:00 2001 From: sanaz Date: Wed, 26 Apr 2023 09:42:57 -0700 Subject: [PATCH 13/34] returns the error returned by validateRange --- nmt.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/nmt.go b/nmt.go index 26c81d0..0598a5c 100644 --- a/nmt.go +++ b/nmt.go @@ -246,6 +246,7 @@ func (n *NamespacedMerkleTree) ProveNamespace(nID namespace.ID) (Proof, error) { } // validateRange validates the range [start, end) against the size of the tree. +// start is inclusive and end is non-inclusive. func (n *NamespacedMerkleTree) validateRange(start, end int) error { if start < 0 || start >= end || end > len(n.leaves) { return ErrInvalidRange @@ -263,7 +264,7 @@ func (n *NamespacedMerkleTree) buildRangeProof(proofStart, proofEnd int) ([][]by // validate the range if err := n.validateRange(proofStart, proofEnd); err != nil { - return nil, ErrInvalidRange + return nil, err } // start, end are indices of leaves in the tree hence they should be within From d11e9c25011fda65c8f1c471a887c5fde6035eff Mon Sep 17 00:00:00 2001 From: sanaz Date: Wed, 26 Apr 2023 11:10:45 -0700 Subject: [PATCH 14/34] adds proof range check --- proof.go | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/proof.go b/proof.go index 3e854af..545253b 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("%w: [%d, %d)", ErrInvalidRange, proof.Start(), proof.End()) + } + // 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 } From bb07d07e4a7aed41ed5083e040a9078a40e4aa51 Mon Sep 17 00:00:00 2001 From: sanaz Date: Wed, 26 Apr 2023 11:11:05 -0700 Subject: [PATCH 15/34] incorporates further tests covering new range check --- proof_test.go | 40 +++++++++++++++++++++++++++++++++++----- 1 file changed, 35 insertions(+), 5 deletions(-) diff --git a/proof_test.go b/proof_test.go index c48c36b..e28d854 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) { From d491b529c5e6d7544b126d4879d52a14c785dd69 Mon Sep 17 00:00:00 2001 From: sanaz Date: Wed, 26 Apr 2023 13:14:14 -0700 Subject: [PATCH 16/34] returns err produced by validateRange --- nmt.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nmt.go b/nmt.go index 0598a5c..2683957 100644 --- a/nmt.go +++ b/nmt.go @@ -171,7 +171,7 @@ func (n *NamespacedMerkleTree) ProveRange(start, end int) (Proof, error) { // TODO: store nodes and re-use the hashes instead recomputing parts of the // tree here if err := n.validateRange(start, end); err != nil { - return NewEmptyRangeProof(isMaxNsIgnored), ErrInvalidRange + return NewEmptyRangeProof(isMaxNsIgnored), err } proof, err := n.buildRangeProof(start, end) if err != nil { From a449d68f702d9423d3a0f50f53ca50e36dd8e86e Mon Sep 17 00:00:00 2001 From: sanaz Date: Wed, 26 Apr 2023 16:12:16 -0700 Subject: [PATCH 17/34] implements the necessary logic to handle empty proofs --- proof.go | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/proof.go b/proof.go index 545253b..db0f440 100644 --- a/proof.go +++ b/proof.go @@ -359,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: From 29007f48475c5c3936b259bb5089cdf2598b1c7e Mon Sep 17 00:00:00 2001 From: sanaz Date: Wed, 26 Apr 2023 16:12:26 -0700 Subject: [PATCH 18/34] develops tests --- proof_test.go | 46 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 46 insertions(+) diff --git a/proof_test.go b/proof_test.go index e28d854..cabaee6 100644 --- a/proof_test.go +++ b/proof_test.go @@ -407,6 +407,52 @@ 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 := namespace.ID{4} // 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()) + + 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: leaves=empty", emptyProof, args{hasher, sampleNID, [][]byte{}, root}, true}, + {"valid empty proof: leaves=non-empty", emptyProof, args{hasher, sampleNID, [][]byte{sampleLeafWithoutNID}, root}, false}, + {"invalid empty proof: leaves = empty", Proof{nodes: [][]byte{sampleNode}}, args{hasher, sampleNID, [][]byte{}, root}, false}, + {"invalid empty proof: leaves != empty", Proof{nodes: [][]byte{sampleNode}}, 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} From 8fdaab2b493bdc0475b3ba5208c18302eda18c22 Mon Sep 17 00:00:00 2001 From: sanaz Date: Wed, 26 Apr 2023 16:21:02 -0700 Subject: [PATCH 19/34] revises the err message --- proof.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/proof.go b/proof.go index db0f440..01034a9 100644 --- a/proof.go +++ b/proof.go @@ -231,7 +231,7 @@ func (proof Proof) VerifyNamespace(h hash.Hash, nID namespace.ID, leaves [][]byt 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("%w: [%d, %d)", ErrInvalidRange, 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: From 409f0f80804f748c04dc93ddaf07403552d1f759 Mon Sep 17 00:00:00 2001 From: sanaz Date: Wed, 26 Apr 2023 16:23:13 -0700 Subject: [PATCH 20/34] extracts the NID from the leaf --- proof_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/proof_test.go b/proof_test.go index cabaee6..fea0dfd 100644 --- a/proof_test.go +++ b/proof_test.go @@ -418,7 +418,7 @@ func TestVerifyInclusion_EmptyProofs(t *testing.T) { 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 := namespace.ID{4} // the NID of the leaf we chose + 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 From 2d504d583d78d7522cc8385dd37b766179b5c776 Mon Sep 17 00:00:00 2001 From: sanaz Date: Wed, 26 Apr 2023 16:32:29 -0700 Subject: [PATCH 21/34] removes an excess line --- proof_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/proof_test.go b/proof_test.go index fea0dfd..63ff10f 100644 --- a/proof_test.go +++ b/proof_test.go @@ -450,7 +450,6 @@ func TestVerifyInclusion_EmptyProofs(t *testing.T) { assert.Equal(t, tt.result, got) }) } - } func TestVerifyNamespace_False(t *testing.T) { From 712ed08a521b254b62a707a3f2d041d3f3a8d83f Mon Sep 17 00:00:00 2001 From: sanaz Date: Thu, 27 Apr 2023 14:18:04 -0700 Subject: [PATCH 22/34] revises tests descriptions --- proof_test.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/proof_test.go b/proof_test.go index 63ff10f..4d21a16 100644 --- a/proof_test.go +++ b/proof_test.go @@ -439,10 +439,10 @@ func TestVerifyInclusion_EmptyProofs(t *testing.T) { args args result bool }{ - {"valid empty proof: leaves=empty", emptyProof, args{hasher, sampleNID, [][]byte{}, root}, true}, - {"valid empty proof: leaves=non-empty", emptyProof, args{hasher, sampleNID, [][]byte{sampleLeafWithoutNID}, root}, false}, - {"invalid empty proof: leaves = empty", Proof{nodes: [][]byte{sampleNode}}, args{hasher, sampleNID, [][]byte{}, root}, false}, - {"invalid empty proof: leaves != empty", Proof{nodes: [][]byte{sampleNode}}, args{hasher, sampleNID, [][]byte{sampleLeafWithoutNID}, root}, false}, + {"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", Proof{nodes: [][]byte{sampleNode}}, args{hasher, sampleNID, [][]byte{}, root}, false}, + {"invalid empty proof and leaves != empty", Proof{nodes: [][]byte{sampleNode}}, args{hasher, sampleNID, [][]byte{sampleLeafWithoutNID}, root}, false}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { From e01ce18a8bd7ca55d5131edd33cb1758c7b7d7d2 Mon Sep 17 00:00:00 2001 From: sanaz Date: Thu, 27 Apr 2023 14:30:35 -0700 Subject: [PATCH 23/34] declares a variable for nonEmptyProof for readability --- proof_test.go | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/proof_test.go b/proof_test.go index 4d21a16..e93c7f6 100644 --- a/proof_test.go +++ b/proof_test.go @@ -427,6 +427,9 @@ func TestVerifyInclusion_EmptyProofs(t *testing.T) { // 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 @@ -441,8 +444,8 @@ func TestVerifyInclusion_EmptyProofs(t *testing.T) { }{ {"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", Proof{nodes: [][]byte{sampleNode}}, args{hasher, sampleNID, [][]byte{}, root}, false}, - {"invalid empty proof and leaves != empty", Proof{nodes: [][]byte{sampleNode}}, 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) { From fc595aaad0892c0c850643c5f1e8f99376347cff Mon Sep 17 00:00:00 2001 From: sanaz Date: Thu, 27 Apr 2023 15:43:01 -0700 Subject: [PATCH 24/34] removes an excess line --- proof_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/proof_test.go b/proof_test.go index e93c7f6..1178bf5 100644 --- a/proof_test.go +++ b/proof_test.go @@ -429,7 +429,7 @@ func TestVerifyInclusion_EmptyProofs(t *testing.T) { // create a non-empty proof nonEmptyProof := Proof{nodes: [][]byte{sampleNode}} - + type args struct { hasher hash.Hash nID namespace.ID From 021adc02ed0fa570e6051ac27eff7eb8b090c6e7 Mon Sep 17 00:00:00 2001 From: sanaz Date: Thu, 27 Apr 2023 15:50:45 -0700 Subject: [PATCH 25/34] replaces manual extraction of min and max NID with proper func calls --- hasher.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/hasher.go b/hasher.go index da9488d..24af94a 100644 --- a/hasher.go +++ b/hasher.go @@ -212,9 +212,9 @@ func (n *Hasher) ValidateNodeFormat(node []byte) (err error) { // namespaced hash values. Otherwise, it panics. func (n *Hasher) validateSiblingsNamespaceOrder(left, right []byte) (err error) { // each NMT node has two namespace IDs for the min and max - totalNamespaceLen := 2 * n.NamespaceLen - leftMaxNs := namespace.ID(left[n.NamespaceLen:totalNamespaceLen]) - rightMinNs := namespace.ID(right[:n.NamespaceLen]) + // totalNamespaceLen := 2 * n.NamespaceLen + leftMaxNs := namespace.ID(MaxNamespace(left, n.NamespaceSize())) + rightMinNs := namespace.ID(MinNamespace(right, n.NamespaceSize())) // check the namespace range of the left and right children if rightMinNs.Less(leftMaxNs) { From 951ad759b0352718a595b5152731644453205373 Mon Sep 17 00:00:00 2001 From: sanaz Date: Thu, 27 Apr 2023 16:05:30 -0700 Subject: [PATCH 26/34] adds node format validation to the validateSiblingsNamespaceOrder --- hasher.go | 10 +++++++--- hasher_test.go | 10 +++++++--- 2 files changed, 14 insertions(+), 6 deletions(-) diff --git a/hasher.go b/hasher.go index 24af94a..02d4dd3 100644 --- a/hasher.go +++ b/hasher.go @@ -207,10 +207,14 @@ func (n *Hasher) ValidateNodeFormat(node []byte) (err error) { // validateSiblingsNamespaceOrder checks whether left and right as two sibling // nodes in an NMT have correct namespace IDs relative to each other, more // specifically, the maximum namespace ID of the left sibling should not exceed -// the minimum namespace ID of the right sibling. It returns ErrUnorderedSiblings error if the check fails. Note that the function assumes -// that the left and right nodes are in correct format, i.e., they are -// namespaced hash values. Otherwise, it panics. +// the minimum namespace ID of the right sibling. It returns ErrUnorderedSiblings error if the check fails. func (n *Hasher) validateSiblingsNamespaceOrder(left, right []byte) (err error) { + if err := n.ValidateNodeFormat(left); err != nil { + return fmt.Errorf("%w: left node is not in the correct format", err) + } + if err := n.ValidateNodeFormat(right); err != nil { + return fmt.Errorf("%w: right node is not in the correct format", err) + } // each NMT node has two namespace IDs for the min and max // totalNamespaceLen := 2 * n.NamespaceLen leftMaxNs := namespace.ID(MaxNamespace(left, n.NamespaceSize())) diff --git a/hasher_test.go b/hasher_test.go index ea15df0..c689adc 100644 --- a/hasher_test.go +++ b/hasher_test.go @@ -277,6 +277,9 @@ func TestHashNode_ChildrenNamespaceRange(t *testing.T) { } func TestValidateSiblingsNamespaceOrder(t *testing.T) { + // create a dummy hash to use as the digest of the left and right child + randHash := createByteSlice(sha256.Size, 0x01) + type children struct { l []byte // namespace hash of the left child with the format of MinNs||MaxNs||h r []byte // namespace hash of the right child with the format of MinNs||MaxNs||h @@ -288,19 +291,20 @@ func TestValidateSiblingsNamespaceOrder(t *testing.T) { children children wantErr bool }{ + { "left.maxNs>right.minNs", 2, - children{[]byte{0, 0, 1, 1}, []byte{0, 0, 1, 1}}, + children{concat([]byte{0, 0, 1, 1}, randHash), concat([]byte{0, 0, 1, 1}, randHash)}, true, }, { "left.maxNs=right.minNs", 2, - children{[]byte{0, 0, 1, 1}, []byte{1, 1, 2, 2}}, + children{concat([]byte{0, 0, 1, 1}, randHash), concat([]byte{1, 1, 2, 2}, randHash)}, false, }, { "left.maxNs Date: Fri, 28 Apr 2023 10:43:29 -0700 Subject: [PATCH 27/34] checks the input range --- nmt.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/nmt.go b/nmt.go index 2683957..5cab24a 100644 --- a/nmt.go +++ b/nmt.go @@ -482,6 +482,9 @@ func (n *NamespacedMerkleTree) MaxNamespace() (namespace.ID, error) { // encompasses the leaves within the range of [start, end). // Any errors returned by this method are irrecoverable and indicate an illegal state of the tree (n). func (n *NamespacedMerkleTree) computeRoot(start, end int) ([]byte, error) { + if err := n.validateRange(start, end); err != nil { + return nil, fmt.Errorf("failed to compute root [%d, %d): %w", start, end, err) + } switch end - start { case 0: rootHash := n.treeHasher.EmptyRoot() From 69d2357f73c591f68b0ae002486d4116b06678f7 Mon Sep 17 00:00:00 2001 From: sanaz Date: Fri, 28 Apr 2023 10:43:38 -0700 Subject: [PATCH 28/34] tests the range --- nmt_test.go | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/nmt_test.go b/nmt_test.go index 0fc7677..bdf32e8 100644 --- a/nmt_test.go +++ b/nmt_test.go @@ -1019,17 +1019,23 @@ func Test_Root_Error(t *testing.T) { // Test_computeRoot_Error tests that the computeRoot method returns an error when the underlying tree is in an invalid state, such as when the leaves are not ordered by namespace ID or when a leaf is corrupt. func Test_computeRoot_Error(t *testing.T) { + nIDSize := 2 + nIDList := []byte{1, 2, 3, 4, 5, 6, 7, 8} + // create an NMT with 8 sequentially namespaced leaves, numbered from 1 to 8. - treeWithCorruptLeafHash := exampleNMT(2, 1, 2, 3, 4, 5, 6, 7, 8) + treeWithCorruptLeafHash := exampleNMT(nIDSize, nIDList...) // corrupt a leaf hash treeWithCorruptLeafHash.leafHashes[4] = treeWithCorruptLeafHash.leafHashes[4][:treeWithCorruptLeafHash.NamespaceSize()-1] // create an NMT with 8 sequentially namespaced leaves, numbered from 1 to 8. - treeWithUnorderedLeaves := exampleNMT(2, 1, 2, 3, 4, 5, 6, 7, 8) + treeWithUnorderedLeaves := exampleNMT(nIDSize, nIDList...) // swap the positions of the 4th and 5th leaves swap(treeWithUnorderedLeaves.leaves, 4, 5) swap(treeWithUnorderedLeaves.leafHashes, 4, 5) + // create an NMT with 8 sequentially namespaced leaves, numbered from 1 to 8. + validTree := exampleNMT(nIDSize, nIDList...) + tests := []struct { name string tree *NamespacedMerkleTree @@ -1037,12 +1043,13 @@ func Test_computeRoot_Error(t *testing.T) { wantErr bool errType error }{ - {"corrupt leaf hash: the entire tree", treeWithCorruptLeafHash, 0, 7, true, ErrInvalidNodeLen}, - {"corrupt leaf: from the corrupt node until the end of the tree", treeWithCorruptLeafHash, 4, 7, true, ErrInvalidNodeLen}, - {"corrupt leaf: the corrupt node and the node to its left", treeWithCorruptLeafHash, 3, 5, true, ErrInvalidNodeLen}, - {"unordered leaves: the entire tree", treeWithUnorderedLeaves, 0, 7, true, ErrUnorderedSiblings}, - {"unordered leaves: the unordered portion", treeWithUnorderedLeaves, 4, 6, true, ErrUnorderedSiblings}, - {"unordered leaves: a portion of the tree containing the unordered leaves", treeWithUnorderedLeaves, 3, 7, true, ErrUnorderedSiblings}, + {"invalid tree with corrupt leaf hash. Query: the entire tree", treeWithCorruptLeafHash, 0, 7, true, ErrInvalidNodeLen}, + {"invalid tree with corrupt leaf. Query: from the corrupt node until the end of the tree", treeWithCorruptLeafHash, 4, 7, true, ErrInvalidNodeLen}, + {"invalid tree with corrupt leaf: Query: the corrupt node and the node to its left", treeWithCorruptLeafHash, 3, 5, true, ErrInvalidNodeLen}, + {"invalid tree with unordered leaves: Query: the entire tree", treeWithUnorderedLeaves, 0, 7, true, ErrUnorderedSiblings}, + {"invalid tree with unordered leaves: Query: the unordered portion", treeWithUnorderedLeaves, 4, 6, true, ErrUnorderedSiblings}, + {"invalid tree with unordered leaves: Query: a portion of the tree containing the unordered leaves", treeWithUnorderedLeaves, 3, 7, true, ErrUnorderedSiblings}, + {"valid tree: Query: start > end", validTree, 3, 1, true, ErrInvalidRange}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { From 2869e6b55ca8883c40fb07cf32033c2e26c07f09 Mon Sep 17 00:00:00 2001 From: sanaz Date: Fri, 28 Apr 2023 10:52:34 -0700 Subject: [PATCH 29/34] deletes an excess line --- hasher_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/hasher_test.go b/hasher_test.go index c689adc..394355c 100644 --- a/hasher_test.go +++ b/hasher_test.go @@ -291,7 +291,6 @@ func TestValidateSiblingsNamespaceOrder(t *testing.T) { children children wantErr bool }{ - { "left.maxNs>right.minNs", 2, children{concat([]byte{0, 0, 1, 1}, randHash), concat([]byte{0, 0, 1, 1}, randHash)}, From a35a4e2a00c4831a193485c452a82c02e1f7b3e2 Mon Sep 17 00:00:00 2001 From: sanaz Date: Fri, 28 Apr 2023 11:08:10 -0700 Subject: [PATCH 30/34] considers start=end as valid --- nmt.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/nmt.go b/nmt.go index 5cab24a..8bc6c32 100644 --- a/nmt.go +++ b/nmt.go @@ -482,8 +482,10 @@ func (n *NamespacedMerkleTree) MaxNamespace() (namespace.ID, error) { // encompasses the leaves within the range of [start, end). // Any errors returned by this method are irrecoverable and indicate an illegal state of the tree (n). func (n *NamespacedMerkleTree) computeRoot(start, end int) ([]byte, error) { - if err := n.validateRange(start, end); err != nil { - return nil, fmt.Errorf("failed to compute root [%d, %d): %w", start, end, err) + // in computeRoot, start may be equal to end which indicates an empty tree hence empty root. + // Due to this, we should not use validateRange() in which start=end is considered invalid. + if start < 0 || start > end || end > len(n.leaves) { + return nil, fmt.Errorf("failed to compute root [%d, %d): %w", start, end, ErrInvalidRange) } switch end - start { case 0: From 81e86bf595354677643813d5bbee2c629f27ac26 Mon Sep 17 00:00:00 2001 From: sanaz Date: Fri, 28 Apr 2023 14:13:43 -0700 Subject: [PATCH 31/34] implements test for the invalid siblings format --- hasher_test.go | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/hasher_test.go b/hasher_test.go index c689adc..e1b856c 100644 --- a/hasher_test.go +++ b/hasher_test.go @@ -291,7 +291,14 @@ func TestValidateSiblingsNamespaceOrder(t *testing.T) { children children wantErr bool }{ - + {"wrong left node format", 2, + children{concat([]byte{0, 0, 1, 1}, randHash[:len(randHash)-1]), concat([]byte{0, 0, 1, 1}, randHash)}, + true, + }, + {"wrong right node format", 2, + children{concat([]byte{0, 0, 1, 1}, randHash), concat([]byte{0, 0, 1, 1}, randHash[:len(randHash)-1])}, + true, + }, { "left.maxNs>right.minNs", 2, children{concat([]byte{0, 0, 1, 1}, randHash), concat([]byte{0, 0, 1, 1}, randHash)}, From f70d2d787482dbf7310952bb021425f2f6fd9784 Mon Sep 17 00:00:00 2001 From: sanaz Date: Fri, 28 Apr 2023 14:15:46 -0700 Subject: [PATCH 32/34] resolves linter issues --- hasher_test.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/hasher_test.go b/hasher_test.go index e1b856c..0a7ed6d 100644 --- a/hasher_test.go +++ b/hasher_test.go @@ -291,11 +291,13 @@ func TestValidateSiblingsNamespaceOrder(t *testing.T) { children children wantErr bool }{ - {"wrong left node format", 2, + { + "wrong left node format", 2, children{concat([]byte{0, 0, 1, 1}, randHash[:len(randHash)-1]), concat([]byte{0, 0, 1, 1}, randHash)}, true, }, - {"wrong right node format", 2, + { + "wrong right node format", 2, children{concat([]byte{0, 0, 1, 1}, randHash), concat([]byte{0, 0, 1, 1}, randHash[:len(randHash)-1])}, true, }, From e446d768cdfcfa747eedddd3b029d21c2e78a2a9 Mon Sep 17 00:00:00 2001 From: sanaz Date: Mon, 1 May 2023 15:20:14 -0700 Subject: [PATCH 33/34] edits comments and tests descriptions --- nmt.go | 2 +- nmt_test.go | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/nmt.go b/nmt.go index 8bc6c32..9f52d03 100644 --- a/nmt.go +++ b/nmt.go @@ -483,7 +483,7 @@ func (n *NamespacedMerkleTree) MaxNamespace() (namespace.ID, error) { // Any errors returned by this method are irrecoverable and indicate an illegal state of the tree (n). func (n *NamespacedMerkleTree) computeRoot(start, end int) ([]byte, error) { // in computeRoot, start may be equal to end which indicates an empty tree hence empty root. - // Due to this, we should not use validateRange() in which start=end is considered invalid. + // Due to this, we need to perform custom range check instead of using validateRange() in which start=end is considered invalid. if start < 0 || start > end || end > len(n.leaves) { return nil, fmt.Errorf("failed to compute root [%d, %d): %w", start, end, ErrInvalidRange) } diff --git a/nmt_test.go b/nmt_test.go index bdf32e8..060b5df 100644 --- a/nmt_test.go +++ b/nmt_test.go @@ -1045,11 +1045,11 @@ func Test_computeRoot_Error(t *testing.T) { }{ {"invalid tree with corrupt leaf hash. Query: the entire tree", treeWithCorruptLeafHash, 0, 7, true, ErrInvalidNodeLen}, {"invalid tree with corrupt leaf. Query: from the corrupt node until the end of the tree", treeWithCorruptLeafHash, 4, 7, true, ErrInvalidNodeLen}, - {"invalid tree with corrupt leaf: Query: the corrupt node and the node to its left", treeWithCorruptLeafHash, 3, 5, true, ErrInvalidNodeLen}, - {"invalid tree with unordered leaves: Query: the entire tree", treeWithUnorderedLeaves, 0, 7, true, ErrUnorderedSiblings}, - {"invalid tree with unordered leaves: Query: the unordered portion", treeWithUnorderedLeaves, 4, 6, true, ErrUnorderedSiblings}, - {"invalid tree with unordered leaves: Query: a portion of the tree containing the unordered leaves", treeWithUnorderedLeaves, 3, 7, true, ErrUnorderedSiblings}, - {"valid tree: Query: start > end", validTree, 3, 1, true, ErrInvalidRange}, + {"invalid tree with corrupt leaf. Query: the corrupt node and the node to its left", treeWithCorruptLeafHash, 3, 5, true, ErrInvalidNodeLen}, + {"invalid tree with unordered leaves. Query: the entire tree", treeWithUnorderedLeaves, 0, 7, true, ErrUnorderedSiblings}, + {"invalid tree with unordered leaves. Query: the unordered portion", treeWithUnorderedLeaves, 4, 6, true, ErrUnorderedSiblings}, + {"invalid tree with unordered leaves. Query: a portion of the tree containing the unordered leaves", treeWithUnorderedLeaves, 3, 7, true, ErrUnorderedSiblings}, + {"valid tree. Query: start > end", validTree, 3, 1, true, ErrInvalidRange}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { From eecd4250ff9072c454e3dff5be3dec96f2895095 Mon Sep 17 00:00:00 2001 From: sanaz Date: Tue, 2 May 2023 14:40:03 -0700 Subject: [PATCH 34/34] tests for start less than zero and end larger than the tree size --- nmt_test.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/nmt_test.go b/nmt_test.go index 060b5df..1acb0b2 100644 --- a/nmt_test.go +++ b/nmt_test.go @@ -1049,7 +1049,9 @@ func Test_computeRoot_Error(t *testing.T) { {"invalid tree with unordered leaves. Query: the entire tree", treeWithUnorderedLeaves, 0, 7, true, ErrUnorderedSiblings}, {"invalid tree with unordered leaves. Query: the unordered portion", treeWithUnorderedLeaves, 4, 6, true, ErrUnorderedSiblings}, {"invalid tree with unordered leaves. Query: a portion of the tree containing the unordered leaves", treeWithUnorderedLeaves, 3, 7, true, ErrUnorderedSiblings}, + {"valid tree. Query: start < 0", validTree, -1, 1, true, ErrInvalidRange}, {"valid tree. Query: start > end", validTree, 3, 1, true, ErrInvalidRange}, + {"valid tree. Query: end > total number of leaves", validTree, 3, len(validTree.leaves) + 1, true, ErrInvalidRange}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) {