shares: NamespacePaddingShare can take advantage of Go's memory guarantees to zero memory and use make([]byte, LEN) instead of bytes.Repeat([]byte{0}, LEN) which is less efficient

[odeke-em]

Summary of Bug

While auditing this repository I noticed these code snippets https://github.com/celestiaorg/go-square/blob/4135f359b20ddd97b6cb74ecc831f70ffb73f8a5/shares/padding.go#L23

and

https://github.com/celestiaorg/go-square/blob/4135f359b20ddd97b6cb74ecc831f70ffb73f8a5/shares/utils.go#L34-L35

Insights

Those code snippets try to create zero byte slices of LEN. Go guarantees that all memory shall be zeroed per https://go.dev/ref/spec#The_zero_value and even better, make is made much more efficient than bytes.Repeat and quick benchmarks can prove this

Benchmarks

diff --git a/shares/padding.go b/shares/padding.go
index e0d905c..3c8d010 100644
--- a/shares/padding.go
+++ b/shares/padding.go
@@ -1,7 +1,6 @@
 package shares

 import (
-   "bytes"
    "errors"

    "github.com/celestiaorg/go-square/namespace"
@@ -20,7 +19,7 @@ func NamespacePaddingShare(ns namespace.Namespace, shareVersion uint8) (Share, e
    if err := b.WriteSequenceLen(0); err != nil {
        return Share{}, err
    }
-   padding := bytes.Repeat([]byte{0}, FirstSparseShareContentSize)
+   padding := make([]byte, FirstSparseShareContentSize)
    b.AddData(padding)

    share, err := b.Build()

with this benchmark

package shares

import (
    "bytes"
    "testing"

    "github.com/celestiaorg/go-square/namespace"
)

var sink any = nil

func pad(tb testing.TB) any {
    ns1 = namespace.MustNewV0(bytes.Repeat([]byte{1}, namespace.NamespaceVersionZeroIDSize))
    got, err := NamespacePaddingShare(ns1, ShareVersionZero)
    if err != nil {
        tb.Fatal(err)
    }
    return got
}

func BenchmarkPaddingVsRepeat(b *testing.B) {
    b.ReportAllocs()

    for i := 0; i < b.N; i++ {
        sink = pad(b)
    }

    if sink == nil {
        b.Fatal("benchmark did not run!")
    }
}

which shows the improvements

$ benchstat before.txt after.txt 
name               old time/op    new time/op    delta
PaddingVsRepeat-8     674ns ± 1%     538ns ± 0%  -20.22%  (p=0.000 n=9+9)

name               old alloc/op   new alloc/op   delta
PaddingVsRepeat-8    1.31kB ± 0%    0.83kB ± 0%  -36.59%  (p=0.000 n=10+10)

name               old allocs/op  new allocs/op  delta
PaddingVsRepeat-8      12.0 ± 0%      11.0 ± 0%   -8.33%  (p=0.000 n=10+10)

/cc @elias-orijtech @liamsi

Version

4135f359b20ddd97b6cb74ecc831f70ffb73f8a5

Steps to Reproduce

Please see the details above

For Admin Use

• [ ] Not duplicate issue
• [ ] Appropriate labels applied
• [ ] Appropriate contributors tagged
• [ ] Contributor assigned/self-assigned