From 622cc16ea5d9c219c57d922d39e57f0db0991e71 Mon Sep 17 00:00:00 2001 From: sanaz Date: Thu, 16 Feb 2023 08:32:37 -0800 Subject: [PATCH 01/11] examines the namespace of children in the HashNode --- hasher.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/hasher.go b/hasher.go index dc92989b..97848e6f 100644 --- a/hasher.go +++ b/hasher.go @@ -195,6 +195,13 @@ func (n *Hasher) HashNode(left, right []byte) []byte { leftMinNs, leftMaxNs := left[:n.NamespaceLen], left[n.NamespaceLen:flagLen] rightMinNs, rightMaxNs := right[:n.NamespaceLen], right[n.NamespaceLen:flagLen] + // check the namespace range of the left and right children + nIDRMin := namespace.ID(rightMinNs) + nIDLMax := namespace.ID(leftMaxNs) + if nIDRMin.Less(nIDLMax) { + panic("nodes are out of order: the maximum namespace of the left child is greater than the min namespace of the right child") + } + minNs := min(leftMinNs, rightMinNs) var maxNs []byte if n.ignoreMaxNs && n.precomputedMaxNs.Equal(leftMinNs) { From ed08c8437db3459a53e4e658fb9cf3b13b114fb6 Mon Sep 17 00:00:00 2001 From: sanaz Date: Thu, 16 Feb 2023 08:39:57 -0800 Subject: [PATCH 02/11] adds unit test for the correctness of namespace range check in the HashNode --- hasher_test.go | 45 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/hasher_test.go b/hasher_test.go index f46c0d19..61290db7 100644 --- a/hasher_test.go +++ b/hasher_test.go @@ -245,3 +245,48 @@ func TestNamespaceHasherSum(t *testing.T) { }) } } + +func TestHashNodeChildrenNamespaceRange(t *testing.T) { + + 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 + } + + tests := []struct { + name string + nidLen namespace.IDSize + children children + panic bool // whether the test should panic or nor + }{ + { + "left.maxNs > right.minNs", 2, + children{[]byte{0, 0, 1, 1}, []byte{0, 0, 1, 1}}, + true, // this test case should panic since in an ordered NMT, left.maxNs cannot be greater than right.minNs + }, + { + "left.maxNs = right.minNs", 2, + children{[]byte{0, 0, 1, 1}, []byte{1, 1, 2, 2}}, + false, + }, + { + "left.maxNs < right.minNs", 2, + children{[]byte{0, 0, 1, 1}, []byte{2, 2, 3, 3}}, + false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + defer func() { + gotPanic := false + if r := recover(); r != nil { // here we check whether panic happened + gotPanic = true + } + assert.Equal(t, tt.panic, gotPanic) + }() + n := NewNmtHasher(sha256.New(), tt.nidLen, false) + n.HashNode(tt.children.l, tt.children.r) + }) + } + +} From 27d37636c4c18ccb75c0398f1c88bae95b5df67e Mon Sep 17 00:00:00 2001 From: sanaz Date: Thu, 16 Feb 2023 10:25:49 -0800 Subject: [PATCH 03/11] removes spaces from the test titles --- hasher_test.go | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/hasher_test.go b/hasher_test.go index 61290db7..c601b0cc 100644 --- a/hasher_test.go +++ b/hasher_test.go @@ -246,8 +246,7 @@ func TestNamespaceHasherSum(t *testing.T) { } } -func TestHashNodeChildrenNamespaceRange(t *testing.T) { - +func TestHashNode_ChildrenNamespaceRange(t *testing.T) { 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 @@ -260,17 +259,17 @@ func TestHashNodeChildrenNamespaceRange(t *testing.T) { panic bool // whether the test should panic or nor }{ { - "left.maxNs > right.minNs", 2, + "left.maxNs>right.minNs", 2, children{[]byte{0, 0, 1, 1}, []byte{0, 0, 1, 1}}, true, // this test case should panic since in an ordered NMT, left.maxNs cannot be greater than right.minNs }, { - "left.maxNs = right.minNs", 2, + "left.maxNs=right.minNs", 2, children{[]byte{0, 0, 1, 1}, []byte{1, 1, 2, 2}}, false, }, { - "left.maxNs < right.minNs", 2, + "left.maxNs Date: Thu, 16 Feb 2023 10:26:15 -0800 Subject: [PATCH 04/11] adds two intermediate variables to hold the left and right children --- hasher.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/hasher.go b/hasher.go index 97848e6f..36e36739 100644 --- a/hasher.go +++ b/hasher.go @@ -123,7 +123,9 @@ func (n *Hasher) Sum([]byte) []byte { case NodePrefix: flagLen := int(n.NamespaceLen) * 2 sha256Len := n.baseHasher.Size() - return n.HashNode(n.data[:flagLen+sha256Len], n.data[flagLen+sha256Len:]) + leftChild := n.data[:flagLen+sha256Len] + rightChild := n.data[flagLen+sha256Len:] + return n.HashNode(leftChild, rightChild) default: panic("nmt node type wasn't set") } From 72a99b40af6cc6d3075201244c692a24626d791f Mon Sep 17 00:00:00 2001 From: sanaz Date: Thu, 16 Feb 2023 10:39:47 -0800 Subject: [PATCH 05/11] picks more clear variable names --- hasher.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/hasher.go b/hasher.go index 36e36739..bf366b5b 100644 --- a/hasher.go +++ b/hasher.go @@ -198,9 +198,9 @@ func (n *Hasher) HashNode(left, right []byte) []byte { rightMinNs, rightMaxNs := right[:n.NamespaceLen], right[n.NamespaceLen:flagLen] // check the namespace range of the left and right children - nIDRMin := namespace.ID(rightMinNs) - nIDLMax := namespace.ID(leftMaxNs) - if nIDRMin.Less(nIDLMax) { + RMinNID := namespace.ID(rightMinNs) + LMaxNID := namespace.ID(leftMaxNs) + if RMinNID.Less(LMaxNID) { panic("nodes are out of order: the maximum namespace of the left child is greater than the min namespace of the right child") } From 19d349fa1359abfaf1d038152356a0a96482335e Mon Sep 17 00:00:00 2001 From: sanaz Date: Thu, 16 Feb 2023 10:40:37 -0800 Subject: [PATCH 06/11] fixes var name capitalization --- hasher.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/hasher.go b/hasher.go index bf366b5b..fd9b13b4 100644 --- a/hasher.go +++ b/hasher.go @@ -198,9 +198,9 @@ func (n *Hasher) HashNode(left, right []byte) []byte { rightMinNs, rightMaxNs := right[:n.NamespaceLen], right[n.NamespaceLen:flagLen] // check the namespace range of the left and right children - RMinNID := namespace.ID(rightMinNs) - LMaxNID := namespace.ID(leftMaxNs) - if RMinNID.Less(LMaxNID) { + rightMinNID := namespace.ID(rightMinNs) + leftMaxNID := namespace.ID(leftMaxNs) + if rightMinNID.Less(leftMaxNID) { panic("nodes are out of order: the maximum namespace of the left child is greater than the min namespace of the right child") } From 7caa9c88edf8cae5ac98b4bf749fe383bcb4d753 Mon Sep 17 00:00:00 2001 From: sanaz Date: Thu, 16 Feb 2023 10:59:20 -0800 Subject: [PATCH 07/11] removes an invalid testcase --- hasher_test.go | 8 -------- 1 file changed, 8 deletions(-) diff --git a/hasher_test.go b/hasher_test.go index c601b0cc..86d022d3 100644 --- a/hasher_test.go +++ b/hasher_test.go @@ -86,14 +86,6 @@ func Test_namespacedTreeHasher_HashNode(t *testing.T) { sum(crypto.SHA256, []byte{NodePrefix}, []byte{0, 0, 0, 0}, []byte{0, 0, 1, 1})..., ), }, - { - "leftmin==rightmin && leftmax>rightmax", 2, - children{[]byte{0, 0, 1, 1}, []byte{0, 0, 0, 1}}, - append( - []byte{0, 0, 1, 1}, - sum(crypto.SHA256, []byte{NodePrefix}, []byte{0, 0, 1, 1}, []byte{0, 0, 0, 1})..., - ), - }, // XXX: can this happen in practice? or is this an invalid state? { "leftmin>rightmin && leftmax Date: Thu, 16 Feb 2023 11:00:53 -0800 Subject: [PATCH 08/11] renames panic to wantPanic for consistency with other tests --- hasher_test.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/hasher_test.go b/hasher_test.go index 86d022d3..23e4a827 100644 --- a/hasher_test.go +++ b/hasher_test.go @@ -245,10 +245,10 @@ func TestHashNode_ChildrenNamespaceRange(t *testing.T) { } tests := []struct { - name string - nidLen namespace.IDSize - children children - panic bool // whether the test should panic or nor + name string + nidLen namespace.IDSize + children children + wantPanic bool // whether the test should panic or nor }{ { "left.maxNs>right.minNs", 2, @@ -273,7 +273,7 @@ func TestHashNode_ChildrenNamespaceRange(t *testing.T) { if r := recover(); r != nil { // here we check whether panic happened gotPanic = true } - assert.Equal(t, tt.panic, gotPanic) + assert.Equal(t, tt.wantPanic, gotPanic) }() n := NewNmtHasher(sha256.New(), tt.nidLen, false) n.HashNode(tt.children.l, tt.children.r) From e7415b7956421365459b0e3b37340c9bc4f834fc Mon Sep 17 00:00:00 2001 From: sanaz Date: Tue, 21 Feb 2023 12:50:24 -0800 Subject: [PATCH 09/11] explains when HashNode can panic --- hasher.go | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/hasher.go b/hasher.go index 90fcd095..268049a9 100644 --- a/hasher.go +++ b/hasher.go @@ -137,15 +137,19 @@ func (n *Hasher) HashLeaf(leaf []byte) []byte { // HashNode calculates a namespaced hash of a node using the supplied left and // right children. The input values, "left" and "right," are namespaced hash -// values with the format "minNID || maxNID || hash." By default, the normal -// namespace hash calculation is followed, which is "res = min(left.minNID, -// right.minNID) || max(left.maxNID, right.maxNID) || H(NodePrefix, left, -// right)". "res" refers to the return value of the HashNode. However, if the -// "ignoreMaxNs" property of the Hasher is set to true, the calculation of the -// namespace ID range of the node slightly changes. In this case, when setting -// the upper range, the maximum possible namespace ID (i.e., -// 2^NamespaceIDSize-1) should be ignored if possible. This is achieved by -// taking the maximum value among the namespace IDs available in the range of +// values with the format "minNID || maxNID || hash." The HashNode function may +// panic if the inputs provided are invalid, i.e., when left and right are not +// in the namespaced hash format or when left.maxNID is greater than +// right.minNID. To prevent panicking, it is advisable to check these criteria +// before calling the HashNode function. +// By default, the normal namespace hash calculation is followed, which is "res +// = min(left.minNID, right.minNID) || max(left.maxNID, right.maxNID) || +// H(NodePrefix, left, right)". "res" refers to the return value of the +// HashNode. However, if the "ignoreMaxNs" property of the Hasher is set to +// true, the calculation of the namespace ID range of the node slightly changes. +// In this case, when setting the upper range, the maximum possible namespace ID +// (i.e., 2^NamespaceIDSize-1) should be ignored if possible. This is achieved +// by taking the maximum value among the namespace IDs available in the range of // its left and right children (i.e., max(left.minNID, left.maxNID , // right.minNID, right.maxNID)), which is not equal to the maximum possible // namespace ID value. If such a namespace ID does not exist, the maximum NID is From 887c86fd1fb02f422af3513d8a801ab628307f72 Mon Sep 17 00:00:00 2001 From: sanaz Date: Tue, 21 Feb 2023 12:56:34 -0800 Subject: [PATCH 10/11] fixes formatting issue causing CI failure --- hasher.go | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/hasher.go b/hasher.go index 268049a9..d87ded99 100644 --- a/hasher.go +++ b/hasher.go @@ -141,19 +141,19 @@ func (n *Hasher) HashLeaf(leaf []byte) []byte { // panic if the inputs provided are invalid, i.e., when left and right are not // in the namespaced hash format or when left.maxNID is greater than // right.minNID. To prevent panicking, it is advisable to check these criteria -// before calling the HashNode function. -// By default, the normal namespace hash calculation is followed, which is "res -// = min(left.minNID, right.minNID) || max(left.maxNID, right.maxNID) || -// H(NodePrefix, left, right)". "res" refers to the return value of the -// HashNode. However, if the "ignoreMaxNs" property of the Hasher is set to -// true, the calculation of the namespace ID range of the node slightly changes. -// In this case, when setting the upper range, the maximum possible namespace ID -// (i.e., 2^NamespaceIDSize-1) should be ignored if possible. This is achieved -// by taking the maximum value among the namespace IDs available in the range of -// its left and right children (i.e., max(left.minNID, left.maxNID , -// right.minNID, right.maxNID)), which is not equal to the maximum possible -// namespace ID value. If such a namespace ID does not exist, the maximum NID is -// calculated as normal, i.e., "res.maxNID = max(left.maxNID , right.maxNID). +// before calling the HashNode function. By default, the normal namespace hash +// calculation is followed, which is "res = min(left.minNID, right.minNID) || +// max(left.maxNID, right.maxNID) || H(NodePrefix, left, right)". "res" refers +// to the return value of the HashNode. However, if the "ignoreMaxNs" property +// of the Hasher is set to true, the calculation of the namespace ID range of +// the node slightly changes. In this case, when setting the upper range, the +// maximum possible namespace ID (i.e., 2^NamespaceIDSize-1) should be ignored +// if possible. This is achieved by taking the maximum value among the namespace +// IDs available in the range of its left and right children (i.e., +// max(left.minNID, left.maxNID , right.minNID, right.maxNID)), which is not +// equal to the maximum possible namespace ID value. If such a namespace ID does +// not exist, the maximum NID is calculated as normal, i.e., "res.maxNID = +// max(left.maxNID , right.maxNID). func (n *Hasher) HashNode(left, right []byte) []byte { h := n.baseHasher h.Reset() From e81c6ca7f001c803ab1646512d74e52f997341d8 Mon Sep 17 00:00:00 2001 From: sanaz Date: Tue, 21 Feb 2023 13:01:33 -0800 Subject: [PATCH 11/11] fixes gofumpt complain --- hasher_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/hasher_test.go b/hasher_test.go index 4f86f8ea..6f025bc6 100644 --- a/hasher_test.go +++ b/hasher_test.go @@ -231,5 +231,4 @@ func TestHashNode_ChildrenNamespaceRange(t *testing.T) { n.HashNode(tt.children.l, tt.children.r) }) } - }