clean up and comment updates

engine/testutil/nodes.go

@@ -302,7 +302,6 @@ func CollectionNode(t *testing.T, hub *stub.Hub, identity bootstrap.NodeInfo, ro
 		coll, err := collections.ByID(collID)
 		return coll, err
 	}
-
 	providerEngine, err := provider.New(
 		node.Log,
 		node.Metrics,

integration/tests/consensus/inclusion_test.go

@@ -43,8 +43,6 @@ func (is *InclusionSuite) SetupTest() {
 	is.log = unittest.LoggerForTest(is.Suite.T(), zerolog.InfoLevel)
 	is.log.Info().Msgf("================> SetupTest")
 
-	// seed random generator
-
 	// to collect node confiis...
 	var nodeConfigs []testnet.NodeConfig
 

model/flow/address_test.go

@@ -166,8 +166,6 @@ func testAddressConstants(t *testing.T) {
 const invalidCodeWord = uint64(0xab2ae42382900010)
 
 func testAddressGeneration(t *testing.T) {
-	// seed random generator
-
 	// loops in each test
 	const loop = 50
 
@@ -258,8 +256,6 @@ func testAddressGeneration(t *testing.T) {
 }
 
 func testAddressesIntersection(t *testing.T) {
-	// seed random generator
-
 	// loops in each test
 	const loop = 25
 
@@ -326,8 +322,6 @@ func testAddressesIntersection(t *testing.T) {
 }
 
 func testIndexFromAddress(t *testing.T) {
-	// seed random generator
-
 	// loops in each test
 	const loop = 50
 
@@ -366,8 +360,6 @@ func testIndexFromAddress(t *testing.T) {
 }
 
 func TestUint48(t *testing.T) {
-	// seed random generator
-
 	const loop = 50
 	// test consistensy of putUint48 and uint48
 	for i := 0; i < loop; i++ {

model/flow/identity.go

@@ -480,8 +480,8 @@ func (il IdentityList) Sample(size uint) (IdentityList, error) {
 	return dup[:size], nil
 }
 
-// Shuffle non-deterministically randomly shuffles the identity
-// list, returning the shuffled list without modifying the receiver.
+// Shuffle randomly shuffles the identity list (non-deterministic),
+// and returns the shuffled list without modifying the receiver.
 func (il IdentityList) Shuffle() (IdentityList, error) {
 	n := uint(len(il))
 	dup := make([]*Identity, 0, n)

module/mempool/herocache/transactions.go

@@ -18,16 +18,14 @@ type Transactions struct {
 
 // NewTransactions implements a transactions mempool based on hero cache.
 func NewTransactions(limit uint32, logger zerolog.Logger, collector module.HeroCacheMetrics) *Transactions {
-	cache := herocache.NewCache(limit,
-		herocache.DefaultOversizeFactor,
-		heropool.LRUEjection,
-		logger.With().Str("mempool", "transactions").Logger(),
-		collector)
-
 	t := &Transactions{
 		c: stdmap.NewBackend(
 			stdmap.WithBackData(
-				cache)),
+				herocache.NewCache(limit,
+					herocache.DefaultOversizeFactor,
+					heropool.LRUEjection,
+					logger.With().Str("mempool", "transactions").Logger(),
+					collector))),
 	}
 
 	return t

module/mempool/queue/heroQueue.go

@@ -21,16 +21,13 @@ type HeroQueue struct {
 
 func NewHeroQueue(sizeLimit uint32, logger zerolog.Logger, collector module.HeroCacheMetrics,
 ) *HeroQueue {
-
-	cache := herocache.NewCache(
-		sizeLimit,
-		herocache.DefaultOversizeFactor,
-		heropool.NoEjection,
-		logger.With().Str("mempool", "hero-queue").Logger(),
-		collector)
-
 	return &HeroQueue{
-		cache:     cache,
+		cache: herocache.NewCache(
+			sizeLimit,
+			herocache.DefaultOversizeFactor,
+			heropool.NoEjection,
+			logger.With().Str("mempool", "hero-queue").Logger(),
+			collector),
 		sizeLimit: uint(sizeLimit),
 	}
 }

module/mempool/queue/heroQueue_test.go

@@ -61,7 +61,6 @@ func TestHeroQueue_Concurrent(t *testing.T) {
 	q := queue.NewHeroQueue(uint32(sizeLimit), unittest.Logger(), metrics.NewNoopCollector())
 	// initially queue must be zero
 	require.Zero(t, q.Size())
-
 	// initially there should be nothing to pop
 	entity, ok := q.Pop()
 	require.False(t, ok)

module/mempool/queue/heroStore.go

@@ -35,10 +35,8 @@ type HeroStore struct {
 
 func NewHeroStore(sizeLimit uint32, logger zerolog.Logger, collector module.HeroCacheMetrics,
 ) *HeroStore {
-	queue := NewHeroQueue(sizeLimit, logger, collector)
-
 	return &HeroStore{
-		q: queue,
+		q: NewHeroQueue(sizeLimit, logger, collector),
 	}
 }
 

module/mempool/stdmap/backDataHeapBenchmark_test.go

@@ -46,16 +46,14 @@ func BenchmarkArrayBackDataLRU(b *testing.B) {
 	defer debug.SetGCPercent(debug.SetGCPercent(-1)) // disable GC
 	limit := uint(50_000)
 
-	cache := herocache.NewCache(
-		uint32(limit),
-		8,
-		heropool.LRUEjection,
-		unittest.Logger(),
-		metrics.NewNoopCollector())
-
 	backData := stdmap.NewBackend(
 		stdmap.WithBackData(
-			cache),
+			herocache.NewCache(
+				uint32(limit),
+				8,
+				heropool.LRUEjection,
+				unittest.Logger(),
+				metrics.NewNoopCollector())),
 		stdmap.WithLimit(limit))
 
 	entities := unittest.EntityListFixture(uint(100_000_000))

network/p2p/connection/peerManager.go

@@ -89,6 +89,9 @@ func (pm *PeerManager) periodicLoop(ctx irrecoverable.SignalerContext) {
 	r, _ := rand.Uint64n(uint64(pm.peerUpdateInterval.Nanoseconds()))
 	// ignore the error here, if randomness fails `r` would be zero and there will be no delay
 	// for the current node
+	// TODO: treat the error properly instead of swallowing it. In this specific case, `utils/rand`
+	// only errors if there is a randomness system issue. Such issue will cause errors in many
+	// other components.
 	delay := time.Duration(r)
 
 	ticker := time.NewTicker(pm.peerUpdateInterval)

state/protocol/badger/mutator_test.go

@@ -40,10 +40,6 @@ import (
 	"github.com/onflow/flow-go/utils/unittest"
 )
 
-func init() {
-
-}
-
 var participants = unittest.IdentityListFixture(5, unittest.WithAllRoles())
 
 func TestBootstrapValid(t *testing.T) {

state/protocol/badger/snapshot_test.go

@@ -26,10 +26,6 @@ import (
 	"github.com/onflow/flow-go/utils/unittest"
 )
 
-func init() {
-
-}
-
 // TestUnknownReferenceBlock tests queries for snapshots which should be unknown.
 // We use this fixture:
 //   - Root height: 100

storage/badger/cleaner.go

@@ -83,7 +83,12 @@ func (c *Cleaner) gcWorkerRoutine(ctx irrecoverable.SignalerContext, ready compo
 // Therefore GC is run every X seconds, where X is uniformly sampled from [interval, interval*1.2]
 func (c *Cleaner) nextWaitDuration() time.Duration {
 	jitter, err := rand.Uint64n(uint64(c.interval.Nanoseconds() / 5))
-	if err != nil { // if randomness fails, do not use a jitter for this instance.
+	if err != nil {
+		// if randomness fails, do not use a jitter for this instance.
+		// TODO: address the error properly and not swallow it.
+		// In this specific case, `utils/rand` only errors if the system randomness fails
+		// which is a symptom of a wider failure. Many other node components would catch such
+		// a failure.
 		jitter = 0
 	}
 	return time.Duration(c.interval.Nanoseconds() + int64(jitter))

storage/badger/operation/common_test.go

@@ -18,10 +18,6 @@ import (
 	"github.com/onflow/flow-go/utils/unittest"
 )
 
-func init() {
-
-}
-
 type Entity struct {
 	ID uint64
 }