Repository: celestiaorg/nmt
Pull request #103 (2023-02-16 19:35:05)

chore!: remove unused API functions Sha256Namespace8FlaggedLeaf and Sha256Namespace8FlaggedInner from the hasher
## Overview
The following two functions `Sha256Namespace8FlaggedLeaf` and `Sha256Namespace8FlaggedInner` are part of the public APIs, however, they are no longer needed as mentioned by @liamsi in https://github.com/celestiaorg/nmt/issues/87#issuecomment-1355165925. I also verified it against the main branch and the latest release (v0.7.0-rc4@8b9937f) of celestia-node codebase and no usage was found. Likewise, no usage was available in the clestia-core repo.

**Summary of changes:**
- This PR deletes these `Sha256Namespace8FlaggedLeaf` and `Sha256Namespace8FlaggedInner` functions and the relevant tests.
- After performing this update, it was found that there was no longer any use case for the `defaultHasher` in the main hasher file, and it is now only being used in the test file. As a result, it has been moved accordingly.

## 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 (based on the responses from code-owners, it is safe to say there is no mention to these functions in any doc)
- [x] Linked issues closed with keywords

This change is inline with https://github.com/celestiaorg/celestia-app/issues/1296 and https://github.com/celestiaorg/nmt/issues/95.


codecov[bot]: # [Codecov](https://codecov.io/gh/celestiaorg/nmt/pull/103?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=celestiaorg) Report
> Merging [#103](https://codecov.io/gh/celestiaorg/nmt/pull/103?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=celestiaorg) (eb467d9) into [master](https://codecov.io/gh/celestiaorg/nmt/commit/b67b45ed8d5de9d19e3827012db193652e2e7a6d?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=celestiaorg) (b67b45e) will **decrease** coverage by `0.06%`.
> The diff coverage is `n/a`.

```diff
@@            Coverage Diff             @@
##           master     #103      +/-   ##
==========================================
- Coverage   96.27%   96.21%   -0.06%     
==========================================
  Files           6        6              
  Lines         429      423       -6     
==========================================
- Hits          413      407       -6     
  Misses         10       10              
  Partials        6        6              
```


| [Impacted Files](https://codecov.io/gh/celestiaorg/nmt/pull/103?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=celestiaorg) | Coverage Δ | |
|---|---|---|
| [hasher.go](https://codecov.io/gh/celestiaorg/nmt/pull/103?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=celestiaorg#diff-aGFzaGVyLmdv) | `95.71% <ø> (-0.34%)` | :arrow_down: |

Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=celestiaorg). Have a feature suggestion? [Share it here.](https://app.codecov.io/gh/feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=celestiaorg)

rootulp: I updated the PR title b/c when we merge that will be the commit title and I think this PR is technically breaking because it removes functions from the public API 

Patch:
diff --git a/hasher.go b/hasher.go
index dc92989..e3074ba 100644
--- a/hasher.go
+++ b/hasher.go
@@ -2,7 +2,6 @@ package nmt
 
 import (
 	"bytes"
-	"crypto/sha256"
 	"hash"
 
 	"github.com/celestiaorg/nmt/namespace"
@@ -15,42 +14,6 @@ const (
 
 var _ hash.Hash = (*Hasher)(nil)
 
-// defaultHasher uses sha256 as a base-hasher, 8 bytes for the namespace IDs and
-// ignores the maximum possible namespace.
-var defaultHasher = NewNmtHasher(sha256.New(), DefaultNamespaceIDLen, true)
-
-// Sha256Namespace8FlaggedLeaf uses sha256 as a base-hasher, 8 bytes for the
-// namespace IDs and ignores the maximum possible namespace.
-//
-// Sha256Namespace8FlaggedLeaf(namespacedData) results in: ns(rawData) ||
-// ns(rawData) || sha256(LeafPrefix || rawData), where rawData is the leaf's
-// data minus the namespace.ID prefix (namely namespacedData[NamespaceLen:]).
-//
-// Note that different from other cryptographic hash functions, this here makes
-// assumptions on the input: len(namespacedData) >= DefaultNamespaceIDLen has to
-// hold, as the first DefaultNamespaceIDLen bytes are interpreted as the
-// namespace ID). If the input does not fulfil this, we will panic. The output
-// will be of length 2*DefaultNamespaceIDLen+sha256.Size = 48 bytes.
-func Sha256Namespace8FlaggedLeaf(namespacedData []byte) []byte {
-	return defaultHasher.HashLeaf(namespacedData)
-}
-
-// Sha256Namespace8FlaggedInner hashes inner nodes to: minNID || maxNID ||
-// sha256(NodePrefix || leftRight), where leftRight consists of the full left
-// and right child node bytes, including their respective min and max namespace
-// IDs. Hence, the input has to be of size: 48 = 32 + 8 + 8  = sha256.Size +
-// 2*DefaultNamespaceIDLen bytes. If the input does not fulfil this, we will
-// panic. The output will also be of length 2*DefaultNamespaceIDLen+sha256.Size
-// = 48 bytes.
-func Sha256Namespace8FlaggedInner(leftRight []byte) []byte {
-	const flagLen = DefaultNamespaceIDLen * 2
-	sha256Len := defaultHasher.baseHasher.Size()
-	left := leftRight[:flagLen+sha256Len]
-	right := leftRight[flagLen+sha256Len:]
-
-	return defaultHasher.HashNode(left, right)
-}
-
 type Hasher struct {
 	baseHasher   hash.Hash
 	NamespaceLen namespace.IDSize


Test patch:
diff --git a/hasher_test.go b/hasher_test.go
index f46c0d1..e7c7325 100644
--- a/hasher_test.go
+++ b/hasher_test.go
@@ -19,6 +19,10 @@ const (
 	innerSize = 2 * hashSize
 )
 
+// defaultHasher uses sha256 as a base-hasher, 8 bytes for the namespace IDs and
+// ignores the maximum possible namespace.
+var defaultHasher = NewNmtHasher(sha256.New(), DefaultNamespaceIDLen, true)
+
 func Test_namespacedTreeHasher_HashLeaf(t *testing.T) {
 	zeroNID := []byte{0}
 	oneNID := []byte{1}
@@ -114,57 +118,6 @@ func Test_namespacedTreeHasher_HashNode(t *testing.T) {
 	}
 }
 
-func TestSha256Namespace8FlaggedLeaf(t *testing.T) {
-	tests := []struct {
-		name      string
-		data      []byte
-		wantPanic bool
-		wantLen   int
-	}{
-		{"input too short: panic", []byte("smaller"), true, 0},
-		{"input 8 byte: Ok", []byte("8bytesss"), false, 48},
-		{"input greater 8 byte: Ok", []byte("8bytesssSomeNotSoRandData"), false, 48},
-	}
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			if tt.wantPanic {
-				shouldPanic(t, func() {
-					Sha256Namespace8FlaggedLeaf(tt.data)
-				})
-			} else if got := Sha256Namespace8FlaggedLeaf(tt.data); len(got) != tt.wantLen {
-				t.Errorf("len(Sha256Namespace8FlaggedLeaf()) = %v, want %v", got, tt.wantLen)
-			}
-		})
-	}
-}
-
-func TestSha256Namespace8FlaggedInner(t *testing.T) {
-	nilHash := sha256.Sum256(nil)
-	nid1 := []byte("nid01234")
-	nid2 := []byte("nid12345")
-	tests := []struct {
-		name      string
-		data      []byte
-		wantPanic bool
-		wantLen   int
-	}{
-		{"input smaller 48: panic", []byte("smaller48"), true, 0},
-		{"input still too small: panic", append(append(nid1, nid2...), []byte("data")...), true, 0},
-		{"valid input: ok", append(append(append(nid1, nilHash[:]...), nid2...), nilHash[:]...), false, 48},
-	}
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			if tt.wantPanic {
-				shouldPanic(t, func() {
-					Sha256Namespace8FlaggedInner(tt.data)
-				})
-			} else if got := Sha256Namespace8FlaggedInner(tt.data); len(got) != tt.wantLen {
-				t.Errorf("len(Sha256Namespace8FlaggedLeaf()) = %v, want %v", got, tt.wantLen)
-			}
-		})
-	}
-}
-
 func sum(hash crypto.Hash, data ...[]byte) []byte {
 	h := hash.New()
 	for _, d := range data {