minor nmt-wrapper doc addition (#306)

* minor doc addition

* typo

* review feedback: classy docs and new emptyNamespaceID var

Co-authored-by: Ismail Khoffi @liamsi

* add better panic docs

* finish sentence

p2p/ipld/nmt_wrapper.go

@@ -3,6 +3,7 @@ package ipld
 import (
 	"bytes"
 	"crypto/sha256"
+	"fmt"
 
 	"github.com/lazyledger/nmt"
 	"github.com/lazyledger/nmt/namespace"
@@ -11,23 +12,31 @@ import (
 	"github.com/lazyledger/lazyledger-core/types"
 )
 
+// emptyNamepsaceID occurs when a share is empty and indicates that
+var emptyNamespaceID = namespace.ID{0, 0, 0, 0, 0, 0, 0, 0}
+
 // Fulfills the rsmt2d.Tree interface and rsmt2d.TreeConstructorFn function
 var _ rsmt2d.TreeConstructorFn = ErasuredNamespacedMerkleTree{}.Constructor
 var _ rsmt2d.Tree = &ErasuredNamespacedMerkleTree{}
 
 // ErasuredNamespacedMerkleTree wraps NamespaceMerkleTree to conform to the
-// rsmt2d.Tree interface while catering specifically to erasure data. For the
-// first half of the tree, it uses the first DefaultNamespaceIDLen number of
-// bytes of the data pushed to determine the namespace. For the second half, it
-// uses the parity namespace ID
+// rsmt2d.Tree interface while also providing the correct namespaces to the
+// underlying NamespaceMerkleTree. It does this by adding the already included
+// namespace to the first half of the tree, and then uses the parity namespace
+// ID for each share pushed to the second half of the tree. This allows for the
+// namespaces to be included in the erasure data, while also keeping the nmt
+// library sufficiently general
 type ErasuredNamespacedMerkleTree struct {
 	squareSize uint64 // note: this refers to the width of the original square before erasure-coded
 	options    []nmt.Option
 	tree       *nmt.NamespacedMerkleTree
 }
 
-// NewErasuredNamespacedMerkleTree issues a new ErasuredNamespacedMerkleTree
+// NewErasuredNamespacedMerkleTree issues a new ErasuredNamespacedMerkleTree. squareSize must be greater than zero
 func NewErasuredNamespacedMerkleTree(squareSize uint64, setters ...nmt.Option) ErasuredNamespacedMerkleTree {
+	if squareSize == 0 {
+		panic("cannot create a ErasuredNamespacedMerkleTree of squareSize == 0")
+	}
 	tree := nmt.New(sha256.New(), setters...)
 	return ErasuredNamespacedMerkleTree{squareSize: squareSize, options: setters, tree: tree}
 }
@@ -42,22 +51,25 @@ func (w ErasuredNamespacedMerkleTree) Constructor() rsmt2d.Tree {
 // Push adds the provided data to the underlying NamespaceMerkleTree, and
 // automatically uses the first DefaultNamespaceIDLen number of bytes as the
 // namespace unless the data pushed to the second half of the tree. Fulfills the
-// rsmt.Tree interface. NOTE: panics if there's an error pushing to underlying
-// NamespaceMerkleTree or if the tree size is exceeded
+// rsmt.Tree interface. NOTE: panics if an error is encountered while pushing or
+// if the tree size is exceeded.
 func (w *ErasuredNamespacedMerkleTree) Push(data []byte, idx rsmt2d.SquareIndex) {
 	// determine the namespace based on where in the tree we're pushing
 	nsID := make(namespace.ID, types.NamespaceSize)
 
 	if idx.Axis+1 > 2*uint(w.squareSize) || idx.Cell+1 > 2*uint(w.squareSize) {
-		panic("pushed past predetermined square size")
+		panic(fmt.Sprintf("pushed past predetermined square size: boundary at %d index at %+v", 2*w.squareSize, idx))
 	}
 
-	// use the parity namespace if the cell is not in Q0 of the extended datasquare
-	// if the cell is empty it means we got an empty block so we need to use TailPaddingNamespaceID
+	// use the parity namespace if the cell is not in Q0 of the extended
+	// datasquare if the cell is empty it means we got an empty block so we need
+	// to use TailPaddingNamespaceID
 	if idx.Axis+1 > uint(w.squareSize) || idx.Cell+1 > uint(w.squareSize) {
 		copy(nsID, types.ParitySharesNamespaceID)
 	} else {
-		if bytes.Equal(data[:types.NamespaceSize], nsID) {
+		// empty shares use the TailPaddingNamespaceID if the data is empty, so
+		// here we check if the share is empty (namepsace == [0,0,0,0,0,0,0,0])
+		if bytes.Equal(data[:types.NamespaceSize], emptyNamespaceID) {
 			copy(nsID, types.TailPaddingNamespaceID)
 		} else {
 			copy(nsID, data[:types.NamespaceSize])

types/consts.go

@@ -53,8 +53,10 @@ var (
 	// https://github.com/lazyledger/lazyledger-specs/blob/master/specs/consensus.md#constants
 	MaxReservedNamespace = namespace.ID{0, 0, 0, 0, 0, 0, 0, 255}
 	// TailPaddingNamespaceID is the namespace ID for tail padding. All data
-	// with this namespace will not be
-	TailPaddingNamespaceID  = namespace.ID{0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFE}
+	// with this namespace will be ignored
+	TailPaddingNamespaceID = namespace.ID{0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFE}
+
+	// ParitySharesNamespaceID indicates that share contains erasure data
 	ParitySharesNamespaceID = namespace.ID{0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF}
 
 	// change accordingly if another hash.Hash should be used as a base hasher in the NMT: