From a0103f96a3c970acd9cd6e7ba178f886a740dd64 Mon Sep 17 00:00:00 2001 From: Oleg Kovalov Date: Fri, 17 Jan 2025 17:16:23 +0100 Subject: [PATCH 1/3] perf: do less allocs in hasher --- hasher.go | 29 +++++++++++------------------ 1 file changed, 11 insertions(+), 18 deletions(-) diff --git a/hasher.go b/hasher.go index f0d174f..a569188 100644 --- a/hasher.go +++ b/hasher.go @@ -154,10 +154,12 @@ func (n *NmtHasher) BlockSize() int { func (n *NmtHasher) EmptyRoot() []byte { n.baseHasher.Reset() - h := n.baseHasher.Sum(nil) - digest := append(make([]byte, int(n.NamespaceLen)*2), h...) + // make returns a zeroed slice, exactly what we need for the (nID || nID) + zeroSize := int(n.NamespaceLen) * 2 + fullSize := zeroSize + n.baseHasher.Size() - return digest + digest := make([]byte, zeroSize, fullSize) + return n.baseHasher.Sum(digest) } // ValidateLeaf verifies if data is namespaced and returns an error if not. @@ -190,11 +192,8 @@ func (n *NmtHasher) HashLeaf(ndata []byte) ([]byte, error) { minMaxNIDs = append(minMaxNIDs, nID...) // nID minMaxNIDs = append(minMaxNIDs, nID...) // nID || nID - // add LeafPrefix to the ndata - leafPrefixedNData := make([]byte, 0, len(ndata)+1) - leafPrefixedNData = append(leafPrefixedNData, LeafPrefix) - leafPrefixedNData = append(leafPrefixedNData, ndata...) - h.Write(leafPrefixedNData) + h.Write([]byte{LeafPrefix}) //nolint:errcheck + h.Write(ndata) //nolint:errcheck // compute h(LeafPrefix || ndata) and append it to the minMaxNIDs nameSpacedHash := h.Sum(minMaxNIDs) // nID || nID || h(LeafPrefix || ndata) @@ -306,19 +305,13 @@ func (n *NmtHasher) HashNode(left, right []byte) ([]byte, error) { // compute the namespace range of the parent node minNs, maxNs := computeNsRange(lRange.Min, lRange.Max, rRange.Min, rRange.Max, n.ignoreMaxNs, n.precomputedMaxNs) - res := make([]byte, 0, len(minNs)*2) + res := make([]byte, 0, len(minNs)+len(maxNs)+h.Size()) res = append(res, minNs...) res = append(res, maxNs...) - // Note this seems a little faster than calling several Write()s on the - // underlying Hash function (see: - // https://github.com/google/trillian/pull/1503): - data := make([]byte, 0, 1+len(left)+len(right)) - data = append(data, NodePrefix) - data = append(data, left...) - data = append(data, right...) - //nolint:errcheck - h.Write(data) + h.Write([]byte{NodePrefix}) //nolint:errcheck + h.Write(left) //nolint:errcheck + h.Write(right) //nolint:errcheck return h.Sum(res), nil } From 192f6d2eb2020b955edbf09be7d2c3b59874ea77 Mon Sep 17 00:00:00 2001 From: Oleg Kovalov Date: Sat, 18 Jan 2025 09:37:33 +0100 Subject: [PATCH 2/3] review suggestions --- hasher.go | 12 +++++------- hasher_test.go | 32 ++++++++++++++++++++++---------- 2 files changed, 27 insertions(+), 17 deletions(-) diff --git a/hasher.go b/hasher.go index a569188..e7e0a65 100644 --- a/hasher.go +++ b/hasher.go @@ -176,8 +176,6 @@ func (n *NmtHasher) ValidateLeaf(data []byte) (err error) { // ns(ndata) || ns(ndata) || hash(leafPrefix || ndata), where ns(ndata) is the // namespaceID inside the data item namely leaf[:n.NamespaceLen]). Note that for // leaves minNs = maxNs = ns(leaf) = leaf[:NamespaceLen]. HashLeaf can return the ErrInvalidNodeLen error if the input is not namespaced. -// -//nolint:errcheck func (n *NmtHasher) HashLeaf(ndata []byte) ([]byte, error) { h := n.baseHasher h.Reset() @@ -192,8 +190,8 @@ func (n *NmtHasher) HashLeaf(ndata []byte) ([]byte, error) { minMaxNIDs = append(minMaxNIDs, nID...) // nID minMaxNIDs = append(minMaxNIDs, nID...) // nID || nID - h.Write([]byte{LeafPrefix}) //nolint:errcheck - h.Write(ndata) //nolint:errcheck + h.Write([]byte{LeafPrefix}) + h.Write(ndata) // compute h(LeafPrefix || ndata) and append it to the minMaxNIDs nameSpacedHash := h.Sum(minMaxNIDs) // nID || nID || h(LeafPrefix || ndata) @@ -309,9 +307,9 @@ func (n *NmtHasher) HashNode(left, right []byte) ([]byte, error) { res = append(res, minNs...) res = append(res, maxNs...) - h.Write([]byte{NodePrefix}) //nolint:errcheck - h.Write(left) //nolint:errcheck - h.Write(right) //nolint:errcheck + h.Write([]byte{NodePrefix}) + h.Write(left) + h.Write(right) return h.Sum(res), nil } diff --git a/hasher_test.go b/hasher_test.go index 8014b0b..8cc71c2 100644 --- a/hasher_test.go +++ b/hasher_test.go @@ -900,17 +900,29 @@ func TestComputeNsRange(t *testing.T) { // TestEmptyRoot ensures that the empty root is always the same, under the same configuration, regardless of the state of the Hasher. func TestEmptyRoot(t *testing.T) { - nIDSzie := 1 - ignoreMaxNS := true + t.Run("the empty root should match a hard-coded empty root", func(t *testing.T) { + nIDSzie := 1 + ignoreMaxNS := true + + hasher := NewNmtHasher(sha256.New(), namespace.IDSize(nIDSzie), ignoreMaxNS) + got := hasher.EmptyRoot() + want := []byte{0x0, 0x0, 0xe3, 0xb0, 0xc4, 0x42, 0x98, 0xfc, 0x1c, 0x14, 0x9a, 0xfb, 0xf4, 0xc8, 0x99, 0x6f, 0xb9, 0x24, 0x27, 0xae, 0x41, 0xe4, 0x64, 0x9b, 0x93, 0x4c, 0xa4, 0x95, 0x99, 0x1b, 0x78, 0x52, 0xb8, 0x55} + assert.Equal(t, want, got) + }) + + t.Run("empty root should return the same root even if the hasher is modified", func(t *testing.T) { + nIDSzie := 1 + ignoreMaxNS := true - hasher := NewNmtHasher(sha256.New(), namespace.IDSize(nIDSzie), ignoreMaxNS) - expectedEmptyRoot := hasher.EmptyRoot() + hasher := NewNmtHasher(sha256.New(), namespace.IDSize(nIDSzie), ignoreMaxNS) + want := hasher.EmptyRoot() - // perform some operation with the hasher - _, err := hasher.HashNode(createByteSlice(hasher.Size(), 1), createByteSlice(hasher.Size(), 1)) - assert.NoError(t, err) - gotEmptyRoot := hasher.EmptyRoot() + // perform some operation with the hasher + _, err := hasher.HashNode(createByteSlice(hasher.Size(), 1), createByteSlice(hasher.Size(), 1)) + assert.NoError(t, err) + got := hasher.EmptyRoot() - // the empty root should be the same before and after the operation - assert.True(t, bytes.Equal(gotEmptyRoot, expectedEmptyRoot)) + // the empty root should be the same before and after the operation + assert.Equal(t, want, got) + }) } From 7301b8cd25aaffa16dd309cb5af47372ec82173c Mon Sep 17 00:00:00 2001 From: Oleg Kovalov Date: Sat, 18 Jan 2025 10:23:19 +0100 Subject: [PATCH 3/3] rabbit suggestions --- hasher_test.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/hasher_test.go b/hasher_test.go index 8cc71c2..cefc169 100644 --- a/hasher_test.go +++ b/hasher_test.go @@ -901,20 +901,20 @@ func TestComputeNsRange(t *testing.T) { // TestEmptyRoot ensures that the empty root is always the same, under the same configuration, regardless of the state of the Hasher. func TestEmptyRoot(t *testing.T) { t.Run("the empty root should match a hard-coded empty root", func(t *testing.T) { - nIDSzie := 1 - ignoreMaxNS := true + nIDSize := 1 + ignoreMaxNs := true - hasher := NewNmtHasher(sha256.New(), namespace.IDSize(nIDSzie), ignoreMaxNS) + hasher := NewNmtHasher(sha256.New(), namespace.IDSize(nIDSize), ignoreMaxNs) got := hasher.EmptyRoot() want := []byte{0x0, 0x0, 0xe3, 0xb0, 0xc4, 0x42, 0x98, 0xfc, 0x1c, 0x14, 0x9a, 0xfb, 0xf4, 0xc8, 0x99, 0x6f, 0xb9, 0x24, 0x27, 0xae, 0x41, 0xe4, 0x64, 0x9b, 0x93, 0x4c, 0xa4, 0x95, 0x99, 0x1b, 0x78, 0x52, 0xb8, 0x55} assert.Equal(t, want, got) }) t.Run("empty root should return the same root even if the hasher is modified", func(t *testing.T) { - nIDSzie := 1 - ignoreMaxNS := true + nIDSize := 1 + ignoreMaxNs := true - hasher := NewNmtHasher(sha256.New(), namespace.IDSize(nIDSzie), ignoreMaxNS) + hasher := NewNmtHasher(sha256.New(), namespace.IDSize(nIDSize), ignoreMaxNs) want := hasher.EmptyRoot() // perform some operation with the hasher