Apply suggestions from John's code review

Co-authored-by: John Adler <[email protected]>
celestiaorg · Oct 6, 2021 · 2025a38 · 2025a38
1 parent 165f04e
commit 2025a38
Show file tree

Hide file tree

Showing 8 changed files with 19 additions and 17 deletions.
diff --git a/go.mod b/go.mod
@@ -2,7 +2,7 @@ module github.com/celestiaorg/celestia-node
 
 go 1.16
 
-replace github.com/ipfs/go-verifcid => github.com/lazyledger/go-verifcid v0.0.1-lazypatch
+replace github.com/ipfs/go-verifcid => github.com/celestiaorg/go-verifcid v0.0.1-lazypatch
 
 require (
 	github.com/BurntSushi/toml v0.4.1

diff --git a/go.sum b/go.sum
@@ -121,6 +121,8 @@ github.com/celestiaorg/celestia-core v0.0.2-0.20210924001615-488ac31b4b3c h1:5He
 github.com/celestiaorg/celestia-core v0.0.2-0.20210924001615-488ac31b4b3c/go.mod h1:i1ECol3kZLZFIkWUJ+/zW5adfvSA8CCl4w/j2FwPOuc=
 github.com/celestiaorg/go-leopard v0.1.0 h1:28z2EkvKJIez5J9CEaiiUEC+OxalRLtTGJJ1oScfE1g=
 github.com/celestiaorg/go-leopard v0.1.0/go.mod h1:NtO/rjlB8dw2aq7jr06vZFKGvryQcTDXaNHelmPNOAM=
+github.com/celestiaorg/go-verifcid v0.0.1-lazypatch h1:9TSe3w1cmJmbWlweCwCTIZkan7jV8M+KwglXpdD+UG8=
+github.com/celestiaorg/go-verifcid v0.0.1-lazypatch/go.mod h1:kXPYu0XqTNUKWA1h3M95UHjUqBzDwXVVt/RXZDjKJmQ=
 github.com/celestiaorg/merkletree v0.0.0-20210714075610-a84dc3ddbbe4 h1:CJdIpo8n5MFP2MwK0gSRcOVlDlFdQJO1p+FqdxYzmvc=
 github.com/celestiaorg/merkletree v0.0.0-20210714075610-a84dc3ddbbe4/go.mod h1:fzuHnhzj1pUygGz+1ZkB3uQbEUL4htqCGJ4Qs2LwMZA=
 github.com/celestiaorg/nmt v0.7.0 h1:XRYh7F7TH6ewD3Gybk1L1zjyvgkWPt1iGqWtbMtB2Lk=
@@ -581,8 +583,6 @@ github.com/kr/pty v1.1.3/go.mod h1:pFQYn66WHrOpPYNljwOMqo10TkYh1fy3cYio2l3bCsQ=
 github.com/kr/text v0.1.0/go.mod h1:4Jbv+DJW3UT/LiOwJeYQe1efqtUx/iVham/4vfdArNI=
 github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY=
 github.com/kr/text v0.2.0/go.mod h1:eLer722TekiGuMkidMxC/pM04lWEeraHUUmBw8l2grE=
-github.com/lazyledger/go-verifcid v0.0.1-lazypatch h1:jAVwUw+DhuCzx5IcYpFh6d6HWxRRz8nhJ3rQo+vlFAc=
-github.com/lazyledger/go-verifcid v0.0.1-lazypatch/go.mod h1:kXPYu0XqTNUKWA1h3M95UHjUqBzDwXVVt/RXZDjKJmQ=
 github.com/leodido/go-urn v1.1.0/go.mod h1:+cyI34gQWZcE1eQU7NVgKkkzdXDQHr1dBMtdAPozLkw=
 github.com/libp2p/go-addr-util v0.0.1/go.mod h1:4ac6O7n9rIAKB1dnd+s8IbbMXkt+oBpzX4/+RACcnlQ=
 github.com/libp2p/go-addr-util v0.0.2/go.mod h1:Ecd6Fb3yIuLzq4bD7VcywcVSBtefcAwnUISBM3WG15E=

diff --git a/ipld/nmt_adder.go b/ipld/nmt_adder.go
@@ -29,7 +29,8 @@ func NewNmtNodeAdder(ctx context.Context, batch *ipld.Batch) *NmtNodeAdder {
 	}
 }
 
-// Visit can be inserted into an nmt tree to create ipld.Nodes while computing the root
+// Visit is a NodeVisitor that can be used during the creation of a new NMT to
+// create and add ipld.Nodes to the Batch while computing the root of the NMT.
 func (n *NmtNodeAdder) Visit(hash []byte, children ...[]byte) {
 	if n.err != nil {
 		return // protect from further visits if there is an error

diff --git a/ipld/plugin/nmt.go b/ipld/plugin/nmt.go
@@ -35,11 +35,13 @@ const (
 	// DagParserFormatName can be used when putting into the IPLD Dag
 	DagParserFormatName = "extended-square-row-or-col"
 
+	// ShareSize system wide default size for data shares.
+	ShareSize = 256
+
 	// Repeated here to avoid a dependency to the wrapping repo as this makes
 	// it hard to compile and use the plugin against a local ipfs version.
 	// TODO: plugins have config options; make this configurable instead
 	namespaceSize = 8
-	shareSize     = 256
 	// nmtHashSize is the size of a digest created by an NMT in bytes.
 	nmtHashSize = 2*namespaceSize + sha256.Size
 )
@@ -113,7 +115,7 @@ func DataSquareRowOrColumnRawInputParser(r io.Reader, _mhType uint64, _mhLen int
 	)
 
 	for {
-		namespacedLeaf := make([]byte, shareSize+namespaceSize)
+		namespacedLeaf := make([]byte, ShareSize+namespaceSize)
 		if _, err := io.ReadFull(br, namespacedLeaf); err != nil {
 			if err == io.EOF {
 				break
@@ -137,7 +139,7 @@ type nmtNodeCollector struct {
 
 func newNodeCollector() *nmtNodeCollector {
 	// extendedRowOrColumnSize is hardcoded here to avoid importing
-	const extendedRowOrColumnSize = 2 * 128
+	extendedRowOrColumnSize := 2 * 128
 	return &nmtNodeCollector{nodes: make([]ipld.Node, 0, extendedRowOrColumnSize)}
 }
 
@@ -165,10 +167,10 @@ func (n *nmtNodeCollector) visit(hash []byte, children ...[]byte) {
 }
 
 func prependNode(newNode ipld.Node, nodes []ipld.Node) []ipld.Node {
-	nodes = append(nodes, ipld.Node(nil))
-	copy(nodes[1:], nodes)
-	nodes[0] = newNode
-	return nodes
+	prepended := make([]ipld.Node, len(nodes)+1)
+	prepended[0] = newNode
+	copy(prepended[1:], nodes)
+	return prepended
 }
 
 func NmtNodeParser(block blocks.Block) (ipld.Node, error) {

diff --git a/ipld/read.go b/ipld/read.go
@@ -140,7 +140,7 @@ func (sc *shareCounter) retrieveShare(
 		}
 	}
 
-	if len(data) < ShareSize {
+	if len(data) < plugin.ShareSize {
 		return
 	}
 

diff --git a/ipld/share.go b/ipld/share.go
@@ -5,8 +5,6 @@ import "github.com/celestiaorg/nmt/namespace"
 const (
 	// MaxSquareSize is currently the maximum size supported for unerasured data in rsmt2d.ExtendedDataSquare.
 	MaxSquareSize = 128
-	// ShareSize system wide default size for data shares.
-	ShareSize = 256
 	// NamespaceSize is a system wide size for NMT namespaces.
 	// TODO(Wondertan): Should be part of IPLD/NMT plugin
 	NamespaceSize = 8

diff --git a/ipld/test_helpers.go b/ipld/test_helpers.go
@@ -73,7 +73,7 @@ func RandNamespacedShares(t *testing.T, total int) NamespacedShares {
 	shares := make(NamespacedShares, total)
 	for i := 0; i < total; i++ {
 		shares[i].ID = data[i]
-		shares[i].Share = make([]byte, NamespaceSize+ShareSize)
+		shares[i].Share = make([]byte, NamespaceSize+plugin.ShareSize)
 		copy(shares[i].Share[:NamespaceSize], data[i])
 		_, err := mrand.Read(shares[i].Share[NamespaceSize:]) // nolint:gosec // G404: Use of weak random number generator
 		require.NoError(t, err)

diff --git a/ipld/write.go b/ipld/write.go
@@ -20,8 +20,8 @@ func PutData(ctx context.Context, shares [][]byte, adder ipld.NodeAdder) (*rsmt2
 	// create nmt adder wrapping batch adder
 	batchAdder := NewNmtNodeAdder(ctx, ipld.NewBatch(ctx, adder))
 	// create the nmt wrapper to generate row and col commitments
-	squareSize := uint32(math.Sqrt(float64(len(shares))))
-	tree := wrapper.NewErasuredNamespacedMerkleTree(uint64(squareSize), nmt.NodeVisitor(batchAdder.Visit))
+	squareSize := uint64(math.Sqrt(float64(len(shares))))
+	tree := wrapper.NewErasuredNamespacedMerkleTree(squareSize, nmt.NodeVisitor(batchAdder.Visit))
 	// recompute the eds
 	eds, err := rsmt2d.ComputeExtendedDataSquare(shares, rsmt2d.NewRSGF8Codec(), tree.Constructor)
 	if err != nil {
@@ -33,6 +33,7 @@ func PutData(ctx context.Context, shares [][]byte, adder ipld.NodeAdder) (*rsmt2
 	return eds, batchAdder.Commit()
 }
 
+// convertEDStoShares returns the original shares of the given ExtendedDataSquare.
 func convertEDStoShares(eds *rsmt2d.ExtendedDataSquare) [][]byte {
 	origWidth := eds.Width() / 2
 	origShares := make([][]byte, origWidth*origWidth)
-Original file line number
+Diff line change
@@ Expand Up / @@ -140,7 +140,7 @@ func (sc *shareCounter) retrieveShare( @@
     		}
     	}
-    	if len(data) < ShareSize {
+    	if len(data) < plugin.ShareSize {
     		return
     	}
@@ Expand Down @@