chore: width clean, fix builds, more comments

builds again

Author: acud

cmd/bee/cmd/start.go

@@ -25,6 +25,7 @@ import (
 	"github.com/ethersphere/bee/v2/pkg/accesscontrol"
 	"github.com/ethersphere/bee/v2/pkg/bmt"
 	"github.com/ethersphere/bee/v2/pkg/bmtpool"
+	"github.com/ethersphere/bee/v2/pkg/keccak"
 	chaincfg "github.com/ethersphere/bee/v2/pkg/config"
 	"github.com/ethersphere/bee/v2/pkg/crypto"
 	"github.com/ethersphere/bee/v2/pkg/keystore"
@@ -271,14 +272,17 @@ func buildBeeNode(ctx context.Context, c *command, cmd *cobra.Command, logger lo
 
 	useSIMD := c.config.GetBool(optionUseSIMD)
 	if useSIMD {
-		if runtime.GOOS != "linux" {
-			return nil, errors.New("SIMD hashing can only be enabled on linux hosts")
+		if runtime.GOOS != "linux" || runtime.GOARCH != "amd64" {
+			return nil, fmt.Errorf("SIMD hashing requires linux/amd64 (this build is %s/%s)", runtime.GOOS, runtime.GOARCH)
 		}
-		logger.Info("SIMD hashing enabled")
-		bmt.SIMDOptIn = true
+		if !keccak.HasSIMD() {
+			return nil, errors.New("SIMD hashing requires a CPU with AVX2 or AVX-512; this CPU has neither")
+		}
+		bmt.SetSIMDOptIn(true)
 		// Rebuild the global bmtpool instance so the new SIMDOptIn value
 		// is reflected in the pool created for hot-path BMT hashing.
 		bmtpool.Rebuild()
+		logger.Info("SIMD hashing enabled", "batch_width", keccak.BatchWidth(), "avx512", keccak.HasAVX512())
 	}
 
 	b, err := node.NewBee(ctx, c.config.GetString(optionNameP2PAddr), signerConfig.publicKey, signerConfig.signer, networkID, logger, signerConfig.libp2pPrivateKey, signerConfig.pssPrivateKey, signerConfig.session, &node.Options{

pkg/bmt/benchmark_test.go

@@ -27,15 +27,15 @@ func BenchmarkBMT(b *testing.B) {
 			benchmarkRefHasher(b, size)
 		})
 		b.Run(fmt.Sprintf("%v_size_%v", "BMT_Goroutine", size), func(b *testing.B) {
-			prev := bmt.SIMDOptIn
-			bmt.SIMDOptIn = false
-			defer func() { bmt.SIMDOptIn = prev }()
+			prev := bmt.SIMDOptIn()
+			bmt.SetSIMDOptIn(false)
+			defer bmt.SetSIMDOptIn(prev)
 			benchmarkBMT(b, size)
 		})
 		b.Run(fmt.Sprintf("%v_size_%v", "BMT_SIMD", size), func(b *testing.B) {
-			prev := bmt.SIMDOptIn
-			bmt.SIMDOptIn = true
-			defer func() { bmt.SIMDOptIn = prev }()
+			prev := bmt.SIMDOptIn()
+			bmt.SetSIMDOptIn(true)
+			defer bmt.SetSIMDOptIn(prev)
 			benchmarkBMT(b, size)
 		})
 	}

pkg/bmt/bmt.go

@@ -7,6 +7,7 @@ package bmt
 import (
 	"encoding/binary"
 	"hash"
+	"sync/atomic"
 
 	"github.com/ethersphere/bee/v2/pkg/swarm"
 )
@@ -16,9 +17,18 @@ var (
 	zerosection = make([]byte, 64)
 )
 
-// SIMDOptIn is default false, meaning it is disabled by default unless the user turns it on with the
-// necessary CLI flag on the supported systems.
-var SIMDOptIn = false
+// simdOptIn controls whether NewPool and friends return a SIMD-backed BMT pool.
+// Default is false; cmd/bee flips it on startup after parsing --use-simd-hashing.
+// Accessed via SIMDOptIn / SetSIMDOptIn so concurrent reads during startup are
+// race-free.
+var simdOptIn atomic.Bool
+
+// SIMDOptIn reports whether the SIMD hasher has been opted into.
+func SIMDOptIn() bool { return simdOptIn.Load() }
+
+// SetSIMDOptIn sets the SIMD opt-in flag. Intended to be called once during
+// startup before the first NewPool call (cmd/bee calls it after flag parsing).
+func SetSIMDOptIn(b bool) { simdOptIn.Store(b) }
 
 // LengthToSpan creates a binary data span size representation.
 // It is required for calculating the BMT hash.

pkg/bmt/bmt_test.go

@@ -272,39 +272,59 @@ func testHasherCorrectness(h bmt.Hasher, data []byte, n, count int) (err error)
 }
 
 // TestGoroutineSIMDParity verifies that the goroutine and SIMD pools produce
-// identical root hashes for the same input, across a spread of write lengths.
-// The SIMD sub-test is a no-op on platforms where the dispatcher falls back to
-// the goroutine pool (non-linux/amd64 or CPU without AVX2).
+// identical root hashes for the same input across every segment count in
+// testSegmentCounts and a spread of write lengths. Iterating over the full
+// segment-count table exercises the degenerate 1-level and shallow-tree branches
+// of simdHasher.Hash that a fixed 128-segment test would miss.
+//
+// On platforms where the dispatcher falls back to the goroutine pool
+// (non-linux/amd64 or CPU without AVX2/AVX-512), both pools end up as goroutine
+// pools and the test degrades to a goroutine-vs-goroutine comparison.
+//
+// Sub-tests are intentionally not run in parallel: the SIMDOptIn flip is global
+// state and concurrent NewPool calls would see flapping values.
 func TestGoroutineSIMDParity(t *testing.T) {
 	testData := testutil.RandBytesWithSeed(t, 4096, seed)
-	lengths := []int{1, 31, 32, 33, 64, 128, 500, 1024, 2048, 4095, 4096}
 
-	prev := bmt.SIMDOptIn
-	defer func() { bmt.SIMDOptIn = prev }()
+	prev := bmt.SIMDOptIn()
+	defer bmt.SetSIMDOptIn(prev)
 
-	bmt.SIMDOptIn = false
-	gPool := bmt.NewPool(bmt.NewConf(testSegmentCount, 1))
-	bmt.SIMDOptIn = true
-	sPool := bmt.NewPool(bmt.NewConf(testSegmentCount, 1))
+	// Representative write lengths; clamped per segment count below. 0 covers the
+	// empty-input path, and the neighbourhood of section boundaries (31/32/33,
+	// 63/64/65) tickles the padding/fall-through in Hash.
+	lengths := []int{0, 1, 31, 32, 33, 63, 64, 65, 128, 500, 1024, 2048, 4095, 4096}
 
-	for _, n := range lengths {
-		t.Run(fmt.Sprintf("len_%d", n), func(t *testing.T) {
-			gh := gPool.Get()
-			gHash, err := syncHash(gh, testData[:n])
-			gPool.Put(gh)
-			if err != nil {
-				t.Fatal(err)
-			}
+	for _, count := range testSegmentCounts {
+		t.Run(fmt.Sprintf("segments_%d", count), func(t *testing.T) {
+			bmt.SetSIMDOptIn(false)
+			gPool := bmt.NewPool(bmt.NewConf(count, 1))
+			bmt.SetSIMDOptIn(true)
+			sPool := bmt.NewPool(bmt.NewConf(count, 1))
+
+			maxLen := count * hashSize
+			for _, n := range lengths {
+				if n > maxLen {
+					continue
+				}
+				t.Run(fmt.Sprintf("len_%d", n), func(t *testing.T) {
+					gh := gPool.Get()
+					gHash, err := syncHash(gh, testData[:n])
+					gPool.Put(gh)
+					if err != nil {
+						t.Fatal(err)
+					}
 
-			sh := sPool.Get()
-			sHash, err := syncHash(sh, testData[:n])
-			sPool.Put(sh)
-			if err != nil {
-				t.Fatal(err)
-			}
+					sh := sPool.Get()
+					sHash, err := syncHash(sh, testData[:n])
+					sPool.Put(sh)
+					if err != nil {
+						t.Fatal(err)
+					}
 
-			if !bytes.Equal(gHash, sHash) {
-				t.Fatalf("goroutine/simd mismatch at len=%d\n  goroutine: %x\n  simd:      %x", n, gHash, sHash)
+					if !bytes.Equal(gHash, sHash) {
+						t.Fatalf("goroutine/simd mismatch at count=%d len=%d\n  goroutine: %x\n  simd:      %x", count, n, gHash, sHash)
+					}
+				})
 			}
 		})
 	}

pkg/bmt/dispatch_other.go

@@ -7,7 +7,7 @@
 package bmt
 
 // NewPool returns a BMT pool. On this platform only the goroutine implementation
-// is compiled in, so SIMDOptIn is ignored.
+// is compiled in, so SIMDOptIn() is ignored.
 func NewPool(c *Conf) Pool {
 	return newGoroutinePool(c)
 }

pkg/bmt/dispatch_simd.go

@@ -10,27 +10,27 @@ import (
 	"github.com/ethersphere/bee/v2/pkg/keccak"
 )
 
-// NewPool returns a BMT pool. If SIMDOptIn is true and the CPU exposes AVX2 or
-// AVX-512, a SIMD-batched pool is returned. Otherwise the goroutine-based pool
-// is returned (silent fallback).
+// NewPool returns a BMT pool. If SIMDOptIn() is true and the CPU exposes AVX2
+// or AVX-512, a SIMD-batched pool is returned. Otherwise the goroutine-based
+// pool is returned (silent fallback).
 func NewPool(c *Conf) Pool {
-	if SIMDOptIn && keccak.HasSIMD() {
+	if SIMDOptIn() && keccak.HasSIMD() {
 		return newSIMDPool(c)
 	}
 	return newGoroutinePool(c)
 }
 
 // NewHasher returns a standalone (non-pooled) BMT hasher.
 func NewHasher() Hasher {
-	if SIMDOptIn && keccak.HasSIMD() {
+	if SIMDOptIn() && keccak.HasSIMD() {
 		return newSIMDHasher()
 	}
 	return newGoroutineHasher()
 }
 
 // NewPrefixHasher returns a standalone BMT hasher with the given prefix.
 func NewPrefixHasher(prefix []byte) Hasher {
-	if SIMDOptIn && keccak.HasSIMD() {
+	if SIMDOptIn() && keccak.HasSIMD() {
 		return newSIMDPrefixHasher(prefix)
 	}
 	return newGoroutinePrefixHasher(prefix)

pkg/bmt/hasher_simd.go

@@ -89,10 +89,8 @@ func (h *simdHasher) Hash(b []byte) ([]byte, error) {
 	}
 	// zero-fill the tail of the buffer: every leaf section must carry
 	// deterministic bytes, because SIMD batches hash whole sections at a time
-	// without a "is this section occupied?" check.
-	for i := h.size; i < h.maxSize; i++ {
-		h.bmt.buffer[i] = 0
-	}
+	// without a "is this section occupied?" check. clear() lowers to memclr.
+	clear(h.bmt.buffer[h.size:])
 	// degenerate single-level tree: the whole buffer is already one section,
 	// so there is nothing for SIMD to batch — just run the scalar hasher.
 	if len(h.bmt.levels) == 1 {
@@ -111,6 +109,10 @@ func (h *simdHasher) Hash(b []byte) ([]byte, error) {
 		return nil, err
 	}
 	// prepend the span and hash once more to produce the chunk address.
+	// When a prefix is configured, baseHasher() returns a PrefixHasher whose
+	// Reset re-absorbs the prefix, so the final digest is
+	// keccak(prefix || span || rootHash) — the same wrap-per-level rule used
+	// by hashSIMD at every internal level.
 	return doHash(h.baseHasher(), h.span, rootHash)
 }
 
@@ -120,7 +122,10 @@ func (h *simdHasher) HashPadded(b []byte) ([]byte, error) {
 	return h.Hash(b)
 }
 
-// Reset prepares the Hasher for reuse.
+// Reset prepares the Hasher for reuse. The internal data buffer is not zeroed;
+// any stale bytes past h.size are overwritten on the next Write and zero-filled
+// on demand by Hash, so post-Reset inspection of the buffer may still see
+// previous-chunk contents.
 func (h *simdHasher) Reset() {
 	h.size = 0
 	copy(h.span, zerospan)

pkg/bmt/pool_simd.go

@@ -49,14 +49,9 @@ func newSIMDConf(prefix []byte, segmentCount, capacity int) *simdConf {
 		maxSize:      count * segmentSize,
 		depth:        depth,
 		prefix:       prefix,
+		batchWidth:   keccak.BatchWidth(),
 	}
 
-	bw := keccak.BatchWidth()
-	if bw == 0 {
-		bw = 8
-	}
-	c.batchWidth = bw
-
 	zerohashes := make([][]byte, depth+1)
 	zeros := make([]byte, segmentSize)
 	zerohashes[0] = zeros

pkg/bmtpool/bmtpool.go

@@ -16,7 +16,7 @@ import (
 const Capacity = 32
 
 // instance holds the active bmt.Pool. Using an atomic pointer so Rebuild can
-// swap it in after cmd/bee sets bmt.SIMDOptIn during startup, without
+// swap it in after cmd/bee calls bmt.SetSIMDOptIn during startup, without
 // introducing lock contention on the hot Get/Put path.
 var instance atomic.Pointer[bmt.Pool]
 
@@ -27,15 +27,17 @@ func init() {
 	// synctest would otherwise trip "receive on synctest channel from outside
 	// bubble" if the pool were first created inside a bubble.
 	//
-	// SIMDOptIn is still false here (flag hasn't been parsed). cmd/bee calls
-	// Rebuild after flag parsing if the user opted in.
+	// bmt.SIMDOptIn() is still false here (flag hasn't been parsed). cmd/bee
+	// calls Rebuild after flag parsing if the user opted in.
 	p := bmt.NewPool(bmt.NewConf(swarm.BmtBranches, Capacity))
 	instance.Store(&p)
 }
 
 // Rebuild discards the current pool and constructs a new one reading the
-// latest bmt.SIMDOptIn value. Intended to be called once during startup after
-// CLI flag parsing so that --use-simd-hashing takes effect.
+// latest bmt.SIMDOptIn() value. Must be called exactly once during startup
+// after CLI flag parsing and before the first external Get, so that
+// --use-simd-hashing takes effect and no in-flight hashers are referencing the
+// old pool's tree.
 func Rebuild() {
 	p := bmt.NewPool(bmt.NewConf(swarm.BmtBranches, Capacity))
 	instance.Store(&p)

pkg/keccak/keccak.go

@@ -1,19 +1,29 @@
-// Copyright 2024 The Swarm Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style
-// license that can be found in the LICENSE file.
-
 // Package keccak provides legacy Keccak-256 (Ethereum-compatible) hashing
 // with SIMD acceleration via XKCP.
 //
-// On amd64, the package automatically selects between AVX-512 (8-way parallel)
-// and AVX2 (4-way parallel) based on the CPU's capabilities.
+// Sum256x4 uses AVX2 (4-way parallel). Sum256x8 uses AVX-512 (8-way parallel).
+// The caller is responsible for dispatching to the correct function based on
+// CPU capabilities; Sum256x8 will crash on non-AVX-512 hardware.
+//
+// # Equal-length (or nil) input constraint
+//
+// All non-nil inputs passed to Sum256x4 / Sum256x8 in the same call MUST have
+// the same length. This is an intrinsic limit of the batched XKCP primitive:
+// KeccakP1600timesN_PermuteAll_24rounds advances the sponge state of every
+// lane in lockstep, so any lane that has already absorbed its final padding
+// block gets extra, unwanted permutations every time a longer lane absorbs
+// another block — silently corrupting its output.
+//
+// Nil (or zero-length) lanes are allowed for partial batches: they are
+// treated as no-op fillers so the caller can populate N_real < N lanes with
+// real data and ignore the remaining N - N_real output digests. The digests
+// produced for nil lanes are not meaningful and must not be consumed. Mixing
+// distinct non-zero lengths within one call is unsupported and will produce
+// wrong digests for the shorter lanes, even when every individual length
+// would work on its own in an all-same-length call.
 package keccak
 
-import (
-	"encoding/hex"
-
-	"golang.org/x/crypto/sha3"
-)
+import "encoding/hex"
 
 // Hash256 represents a 32-byte Keccak-256 hash
 type Hash256 [32]byte
@@ -32,7 +42,7 @@ func HasAVX512() bool {
 // HasSIMD reports whether any SIMD-accelerated Keccak path is available
 // (AVX2 or AVX-512).
 func HasSIMD() bool {
-	return hasAVX2
+	return hasAVX2 || hasAVX512
 }
 
 // BatchWidth returns the SIMD batch width: 8 for AVX-512, 4 for AVX2, or 0
@@ -47,22 +57,12 @@ func BatchWidth() int {
 	return 0
 }
 
-// Sum256 computes a single Keccak-256 hash (legacy, Ethereum-compatible).
-// Uses the best available implementation.
-func Sum256(data []byte) Hash256 {
-	var out Hash256
-	h := sha3.NewLegacyKeccak256()
-	h.Write(data)
-	copy(out[:], h.Sum(nil))
-	return out
-}
-
 // Sum256x4 computes 4 Keccak-256 hashes in parallel using AVX2.
-// Callers must check HasSIMD() first; invoking without AVX2 panics.
+//
+// All non-nil inputs MUST have the same length; nil (or zero-length) lanes
+// are allowed as partial-batch fillers whose output must be ignored. See the
+// package doc for the rationale.
 func Sum256x4(inputs [4][]byte) [4]Hash256 {
-	if !hasAVX2 {
-		panic("keccak: Sum256x4 requires AVX2; call HasSIMD() first")
-	}
 	var outputs [4]Hash256
 	var inputsCopy [4][]byte
 	copy(inputsCopy[:], inputs[:])
@@ -71,11 +71,12 @@ func Sum256x4(inputs [4][]byte) [4]Hash256 {
 }
 
 // Sum256x8 computes 8 Keccak-256 hashes in parallel using AVX-512.
-// Callers must check HasAVX512() first; invoking without AVX-512 panics.
+// Must only be called on AVX-512-capable hardware.
+//
+// All non-nil inputs MUST have the same length; nil (or zero-length) lanes
+// are allowed as partial-batch fillers whose output must be ignored. See the
+// package doc for the rationale.
 func Sum256x8(inputs [8][]byte) [8]Hash256 {
-	if !hasAVX512 {
-		panic("keccak: Sum256x8 requires AVX-512; call HasAVX512() first")
-	}
 	var outputs [8]Hash256
 	var inputsCopy [8][]byte
 	copy(inputsCopy[:], inputs[:])