feat: ensures consistent use of the assumption that NMT nodes are ordered ascendingly

hasher.go

@@ -17,9 +17,10 @@ const (
 var _ hash.Hash = (*Hasher)(nil)
 
 var (
-	ErrUnorderedSiblings = errors.New("NMT sibling nodes should be ordered lexicographically by namespace IDs")
-	ErrInvalidNodeLen    = errors.New("invalid NMT node size")
-	ErrInvalidLeafLen    = errors.New("invalid NMT leaf size")
+	ErrUnorderedSiblings         = errors.New("NMT sibling nodes should be ordered lexicographically by namespace IDs")
+	ErrInvalidNodeLen            = errors.New("invalid NMT node size")
+	ErrInvalidLeafLen            = errors.New("invalid NMT leaf size")
+	ErrInvalidNodeNamespaceOrder = errors.New("invalid NMT node namespace order")
 )
 
 type Hasher struct {
@@ -201,20 +202,28 @@ func (n *Hasher) ValidateNodeFormat(node []byte) (err error) {
 	if nodeLen != expectedNodeLen {
 		return fmt.Errorf("%w: got: %v, want %v", ErrInvalidNodeLen, nodeLen, expectedNodeLen)
 	}
+	// check the namespace order
+	minNID := namespace.ID(MinNamespace(node, n.NamespaceSize()))
+	maxNID := namespace.ID(MaxNamespace(node, n.NamespaceSize()))
+	if maxNID.Less(minNID) {
+		return fmt.Errorf("%w: max namespace ID %d is less than min namespace ID %d ", ErrInvalidNodeNamespaceOrder, maxNID, minNID)
+	}
 	return nil
 }
 
 // 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) {
-	// 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])
+	if err := n.ValidateNodeFormat(left); err != nil {
+		return fmt.Errorf("%w: left node does not match the namesapce hash format", err)
+	}
+	if err := n.ValidateNodeFormat(right); err != nil {
+		return fmt.Errorf("%w: right node does not match the namesapce hash format", err)
+	}
+	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) {
@@ -274,14 +283,10 @@ func (n *Hasher) HashNode(left, right []byte) ([]byte, error) {
 	leftMinNs, leftMaxNs := left[:n.NamespaceLen], left[n.NamespaceLen:flagLen]
 	rightMinNs, rightMaxNs := right[:n.NamespaceLen], right[n.NamespaceLen:flagLen]
 
-	minNs := min(leftMinNs, rightMinNs)
-	var maxNs []byte
-	if n.ignoreMaxNs && n.precomputedMaxNs.Equal(leftMinNs) {
-		maxNs = n.precomputedMaxNs
-	} else if n.ignoreMaxNs && n.precomputedMaxNs.Equal(rightMinNs) {
+	minNs := leftMinNs
+	maxNs := rightMaxNs
+	if n.ignoreMaxNs && n.precomputedMaxNs.Equal(rightMinNs) {
 		maxNs = leftMaxNs
-	} else {
-		maxNs = max(leftMaxNs, rightMaxNs)
 	}
 
 	res := make([]byte, 0)

hasher_test.go

@@ -100,18 +100,6 @@ func Test_namespacedTreeHasher_HashNode(t *testing.T) {
 					concat([]byte{0, 0, 0, 0}, randHash),
 					concat([]byte{0, 0, 1, 1}, randHash))),
 		},
-		// XXX: can this happen in practice? or is this an invalid state?
-		{
-			"leftmin>rightmin && leftmax<rightmax", 2,
-			children{
-				concat([]byte{1, 1, 0, 0}, randHash),
-				concat([]byte{0, 0, 0, 1}, randHash),
-			},
-			concat([]byte{0, 0, 0, 1}, // minNID||maxNID
-				sum(crypto.SHA256, []byte{NodePrefix}, // Hash(NodePrefix||left||right)
-					concat([]byte{1, 1, 0, 0}, randHash),
-					concat([]byte{0, 0, 0, 1}, randHash))),
-		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
@@ -221,7 +209,8 @@ func TestNamespaceHasherSum(t *testing.T) {
 	}
 }
 
-func TestHashNode_ChildrenNamespaceRange(t *testing.T) {
+// TestHashNode tests the HashNode function for cases that it should and should not produce errors.
+func TestHashNode_Error(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 {
@@ -237,7 +226,7 @@ func TestHashNode_ChildrenNamespaceRange(t *testing.T) {
 		errType  error
 	}{
 		{
-			"left.maxNs>right.minNs", 2,
+			"unordered siblings: left.maxNs>right.minNs", 2,
 			children{
 				concat([]byte{0, 0, 1, 1}, randHash),
 				concat([]byte{0, 0, 1, 1}, randHash),
@@ -246,7 +235,7 @@ func TestHashNode_ChildrenNamespaceRange(t *testing.T) {
 			ErrUnorderedSiblings,
 		},
 		{
-			"left.maxNs=right.minNs", 2,
+			"ordered siblings: left.maxNs=right.minNs", 2,
 			children{
 				concat([]byte{0, 0, 1, 1}, randHash),
 				concat([]byte{1, 1, 2, 2}, randHash),
@@ -255,14 +244,32 @@ func TestHashNode_ChildrenNamespaceRange(t *testing.T) {
 			nil,
 		},
 		{
-			"left.maxNs<right.minNs", 2,
+			"ordered siblings: left.maxNs<right.minNs", 2,
 			children{
 				concat([]byte{0, 0, 1, 1}, randHash),
 				concat([]byte{2, 2, 3, 3}, randHash),
 			},
 			false,
 			nil,
 		},
+		{
+			"invalid left sibling format: left.minNs>left.maxNs", 2,
+			children{
+				concat([]byte{2, 2, 0, 0}, randHash),
+				concat([]byte{1, 1, 4, 4}, randHash),
+			},
+			true,
+			ErrInvalidNodeNamespaceOrder,
+		},
+		{
+			"invalid right sibling format: right.minNs>right.maxNs", 2,
+			children{
+				concat([]byte{0, 0, 1, 1}, randHash),
+				concat([]byte{4, 4, 1, 1}, randHash),
+			},
+			true,
+			ErrInvalidNodeNamespaceOrder,
+		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
@@ -276,7 +283,10 @@ func TestHashNode_ChildrenNamespaceRange(t *testing.T) {
 	}
 }
 
-func TestValidateSiblingsNamespaceOrder(t *testing.T) {
+func TestValidateSiblings(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 +298,29 @@ 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{[]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<right.minNs", 2,
-			children{[]byte{0, 0, 1, 1}, []byte{2, 2, 3, 3}},
+			children{concat([]byte{0, 0, 1, 1}, randHash), concat([]byte{2, 2, 3, 3}, randHash)},
 			false,
 		},
 	}
@@ -362,6 +382,33 @@ func TestValidateNodeFormat(t *testing.T) {
 			true,
 			ErrInvalidNodeLen,
 		},
+		{
+			"invalid node: minNS > maxNs",
+			2,
+			[]byte{3, 3},
+			[]byte{1, 1},
+			concat(hashValue),
+			true,
+			ErrInvalidNodeNamespaceOrder,
+		},
+		{
+			"valid node: minNs = maxNs",
+			2,
+			minNID,
+			minNID,
+			concat(hashValue),
+			false,
+			nil,
+		},
+		{
+			"valid node: minNs < maxNs",
+			2,
+			minNID,
+			maxNID,
+			concat(hashValue),
+			false,
+			nil,
+		},
 	}
 
 	for _, tt := range tests {

proof_test.go

@@ -286,13 +286,16 @@ func TestVerifyLeafHashes_Err(t *testing.T) {
 	root, err := nmt.Root()
 	require.NoError(t, err)
 
+	corruptRoot := root[:nmt.NamespaceSize()]
+
 	// create an NMT proof
 	nID5 := namespace.ID{5, 5}
 	proof5, err := nmt.ProveNamespace(nID5)
 	require.NoError(t, err)
 	// corrupt the leafHash so that the proof verification fails during the root computation.
 	// note that the leaf at index 4 has the namespace ID of 5.
-	leafHash5 := nmt.leafHashes[4][:nmt.NamespaceSize()]
+	leafHash5 := nmt.leafHashes[4]
+	corruptLeafHash5 := leafHash5[:nmt.NamespaceSize()]
 
 	// corrupt the leafHash: replace its namespace ID with a different one.
 	nID3 := createByteSlice(nameIDSize, 3)
@@ -339,7 +342,8 @@ func TestVerifyLeafHashes_Err(t *testing.T) {
 		root               []byte
 		wantErr            bool
 	}{
-		{"wrong leafHash: not namespaced", proof5, hasher, true, nID5, [][]byte{leafHash5}, root, true},
+		{"corrupt root", proof5, hasher, true, nID5, [][]byte{leafHash5}, corruptRoot, true},
+		{"wrong leafHash: not namespaced", proof5, hasher, true, nID5, [][]byte{corruptLeafHash5}, 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},
@@ -507,8 +511,8 @@ func TestVerifyNamespace_False(t *testing.T) {
 		args   args
 		result bool
 	}{
-		{"nID size of proof < nID size of VerifyNamespace's nmt hasher", proof4_1, args{hasher, nid4_2, [][]byte{leaf}, root2}, false},
-		{"nID size of proof > nID size of VerifyNamespace's nmt hasher", proof4_2, args{hasher, nid4_1, [][]byte{leaf}, root1}, false},
+		{"nID size of proof.nodes < nID size of VerifyNamespace's nmt hasher", proof4_1, args{hasher, nid4_2, [][]byte{leaf}, root2}, false},
+		{"nID size of proof.nodes > nID size of VerifyNamespace's nmt hasher", proof4_2, args{hasher, nid4_1, [][]byte{leaf}, root1}, false},
 		{"nID size of root < nID size of VerifyNamespace's nmt hasher", proof4_2, args{hasher, nid4_2, [][]byte{leaf}, root1}, false},
 		{"nID size of root > nID size of VerifyNamespace's nmt hasher", proof4_1, args{hasher, nid4_1, [][]byte{leaf}, root2}, false},
 		{"nID size of proof.leafHash < nID size of VerifyNamespace's nmt hasher", absenceProof9_2, args{hasher, nid9_2, [][]byte{}, root2}, false},