[randomness final part] Update math/rand usage

.github/workflows/ci.yml

@@ -66,8 +66,8 @@ jobs:
           cache: true
       - name: Run tidy
         run: make tidy
-      - name: Emulator no relic check
-        run: make emulator-norelic-check
+      - name: code sanity check
+        run: make code-sanity-check
 
   shell-check:
     name: ShellCheck

Makefile

@@ -87,6 +87,23 @@ emulator-norelic-check:
 	# test the fvm package compiles with Relic library disabled (required for the emulator build)
 	cd ./fvm && go test ./... -run=NoTestHasThisPrefix
 
+.SILENT: go-math-rand-check
+go-math-rand-check:
+	# check that the insecure math/rand Go package isn't used by production code.
+	# `exclude` should only specify non production code (test, bench..).
+	# If this check fails, try updating your code by using:
+	#   - "crypto/rand" or "flow-go/utils/rand" for non-deterministic randomness
+	#   - "flow-go/crypto/random" for deterministic randomness 
+	grep --include=\*.go \
+	--exclude=*test* --exclude=*helper* --exclude=*example* --exclude=*fixture* --exclude=*benchmark* --exclude=*profiler* \
+    --exclude-dir=*test* --exclude-dir=*helper* --exclude-dir=*example* --exclude-dir=*fixture* --exclude-dir=*benchmark* --exclude-dir=*profiler* -rnw '"math/rand"'; \
+    if [ $$? -ne 1 ]; then \
+       echo "[Error] Go production code should not use math/rand package"; exit 1; \
+    fi
+
+.PHONY: code-sanity-check
+code-sanity-check: go-math-rand-check emulator-norelic-check
+
 .PHONY: fuzz-fvm
 fuzz-fvm:
 	# run fuzz tests in the fvm package

cmd/bootstrap/cmd/clusters.go

@@ -10,7 +10,6 @@ import (
 	"github.com/onflow/flow-go/model/flow/assignment"
 	"github.com/onflow/flow-go/model/flow/factory"
 	"github.com/onflow/flow-go/model/flow/filter"
-	"github.com/onflow/flow-go/utils/rand"
 )
 
 // Construct random cluster assignment with internal and partner nodes.
@@ -39,12 +38,11 @@ func constructClusterAssignment(partnerNodes, internalNodes []model.NodeInfo) (f
 	}
 
 	// shuffle both collector lists based on a non-deterministic algorithm
-	var err error
-	err = rand.Shuffle(uint(len(partners)), func(i, j uint) { partners[i], partners[j] = partners[j], partners[i] })
+	partners, err := partners.Shuffle()
 	if err != nil {
 		log.Fatal().Err(err).Msg("could not shuffle partners")
 	}
-	err = rand.Shuffle(uint(len(internals)), func(i, j uint) { internals[i], internals[j] = internals[j], internals[i] })
+	internals, err = internals.Shuffle()
 	if err != nil {
 		log.Fatal().Err(err).Msg("could not shuffle internals")
 	}

cmd/scaffold.go

@@ -5,7 +5,6 @@ import (
 	"crypto/x509"
 	"errors"
 	"fmt"
-	"math/rand"
 	"os"
 	"runtime"
 	"strings"
@@ -1773,10 +1772,6 @@ func (fnb *FlowNodeBuilder) Build() (Node, error) {
 }
 
 func (fnb *FlowNodeBuilder) onStart() error {
-
-	// seed random generator
-	rand.Seed(time.Now().UnixNano())
-
 	// init nodeinfo by reading the private bootstrap file if not already set
 	if fnb.NodeID == flow.ZeroID {
 		if err := fnb.initNodeInfo(); err != nil {

consensus/hotstuff/signature/block_signer_decoder_test.go

@@ -140,7 +140,8 @@ func (s *blockSignerDecoderSuite) Test_EpochTransition() {
 	blockView := s.block.Header.View
 	parentView := s.block.Header.ParentView
 	epoch1Committee := s.allConsensus
-	epoch2Committee := s.allConsensus.SamplePct(.8)
+	epoch2Committee, err := s.allConsensus.SamplePct(.8)
+	require.NoError(s.T(), err)
 
 	*s.committee = *hotstuff.NewDynamicCommittee(s.T())
 	s.committee.On("IdentitiesByEpoch", parentView).Return(epoch1Committee, nil).Maybe()

consensus/integration/network_test.go

@@ -160,7 +160,11 @@ func (n *Network) publish(event interface{}, channel channels.Channel, targetIDs
 // Engines attached to the same channel on other nodes. The targeted nodes are selected based on the selector.
 // In this test helper implementation, multicast uses submit method under the hood.
 func (n *Network) multicast(event interface{}, channel channels.Channel, num uint, targetIDs ...flow.Identifier) error {
-	targetIDs = flow.Sample(num, targetIDs...)
+	var err error
+	targetIDs, err = flow.Sample(num, targetIDs...)
+	if err != nil {
+		return fmt.Errorf("sampling failed: %w", err)
+	}
 	return n.submit(event, channel, targetIDs...)
 }
 

engine/access/rpc/backend/backend.go

@@ -370,7 +370,10 @@ func executionNodesForBlockID(
 	}
 
 	// randomly choose upto maxExecutionNodesCnt identities
-	executionIdentitiesRandom := subsetENs.Sample(maxExecutionNodesCnt)
+	executionIdentitiesRandom, err := subsetENs.Sample(maxExecutionNodesCnt)
+	if err != nil {
+		return nil, fmt.Errorf("sampling failed: %w", err)
+	}
 
 	if len(executionIdentitiesRandom) == 0 {
 		return nil, fmt.Errorf("no matching execution node found for block ID %v", blockID)

engine/access/rpc/backend/backend_transactions.go

@@ -129,7 +129,10 @@ func (b *backendTransactions) chooseCollectionNodes(tx *flow.TransactionBody, sa
 	}
 
 	// select a random subset of collection nodes from the cluster to be tried in order
-	targetNodes := txCluster.Sample(sampleSize)
+	targetNodes, err := txCluster.Sample(sampleSize)
+	if err != nil {
+		return nil, fmt.Errorf("sampling failed: %w", err)
+	}
 
 	// collect the addresses of all the chosen collection nodes
 	var targetAddrs = make([]string, len(targetNodes))

engine/common/requester/engine.go

@@ -367,7 +367,11 @@ func (e *Engine) dispatchRequest() (bool, error) {
 			if len(providers) == 0 {
 				return false, fmt.Errorf("no valid providers available")
 			}
-			providerID = providers.Sample(1)[0].NodeID
+			id, err := providers.Sample(1)
+			if err != nil {
+				return false, fmt.Errorf("sampling failed: %w", err)
+			}
+			providerID = id[0].NodeID
 		}
 
 		// add item to list and set retry parameters

engine/execution/computation/query/executor.go

@@ -4,7 +4,6 @@ import (
 	"context"
 	"encoding/hex"
 	"fmt"
-	"math/rand"
 	"strings"
 	"sync"
 	"time"
@@ -18,6 +17,7 @@ import (
 	"github.com/onflow/flow-go/model/flow"
 	"github.com/onflow/flow-go/module"
 	"github.com/onflow/flow-go/utils/debug"
+	"github.com/onflow/flow-go/utils/rand"
 )
 
 const (
@@ -71,7 +71,6 @@ type QueryExecutor struct {
 	vmCtx            fvm.Context
 	derivedChainData *derived.DerivedChainData
 	rngLock          *sync.Mutex
-	rng              *rand.Rand
 }
 
 var _ Executor = &QueryExecutor{}
@@ -92,7 +91,6 @@ func NewQueryExecutor(
 		vmCtx:            vmCtx,
 		derivedChainData: derivedChainData,
 		rngLock:          &sync.Mutex{},
-		rng:              rand.New(rand.NewSource(time.Now().UnixNano())),
 	}
 }
 
@@ -115,8 +113,11 @@ func (e *QueryExecutor) ExecuteScript(
 	// TODO: this is a temporary measure, we could remove this in the future
 	if e.logger.Debug().Enabled() {
 		e.rngLock.Lock()
-		trackerID := e.rng.Uint32()
-		e.rngLock.Unlock()
+		defer e.rngLock.Unlock()
+		trackerID, err := rand.Uint32()
+		if err != nil {
+			return nil, fmt.Errorf("failed to generate trackerID: %w", err)
+		}
 
 		trackedLogger := e.logger.With().Hex("script_hex", script).Uint32("trackerID", trackerID).Logger()
 		trackedLogger.Debug().Msg("script is sent for execution")

engine/verification/requester/requester.go

@@ -331,8 +331,11 @@ func (e *Engine) requestChunkDataPack(request *verification.ChunkDataPackRequest
 	}
 
 	// publishes the chunk data request to the network
-	targetIDs := request.SampleTargets(int(e.requestTargets))
-	err := e.con.Publish(req, targetIDs...)
+	targetIDs, err := request.SampleTargets(int(e.requestTargets))
+	if err != nil {
+		return fmt.Errorf("target sampling failed: %w", err)
+	}
+	err = e.con.Publish(req, targetIDs...)
 	if err != nil {
 		return fmt.Errorf("could not publish chunk data pack request for chunk (id=%s): %w", request.ChunkID, err)
 	}

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

@@ -5,7 +5,6 @@ import (
 	"math/bits"
 	"math/rand"
 	"testing"
-	"time"
 
 	"github.com/onflow/cadence"
 	"github.com/onflow/cadence/runtime/common"
@@ -167,9 +166,6 @@ func testAddressConstants(t *testing.T) {
 const invalidCodeWord = uint64(0xab2ae42382900010)
 
 func testAddressGeneration(t *testing.T) {
-	// seed random generator
-	rand.Seed(time.Now().UnixNano())
-
 	// loops in each test
 	const loop = 50
 
@@ -260,9 +256,6 @@ func testAddressGeneration(t *testing.T) {
 }
 
 func testAddressesIntersection(t *testing.T) {
-	// seed random generator
-	rand.Seed(time.Now().UnixNano())
-
 	// loops in each test
 	const loop = 25
 
@@ -329,9 +322,6 @@ func testAddressesIntersection(t *testing.T) {
 }
 
 func testIndexFromAddress(t *testing.T) {
-	// seed random generator
-	rand.Seed(time.Now().UnixNano())
-
 	// loops in each test
 	const loop = 50
 
@@ -370,9 +360,6 @@ func testIndexFromAddress(t *testing.T) {
 }
 
 func TestUint48(t *testing.T) {
-	// seed random generator
-	rand.Seed(time.Now().UnixNano())
-
 	const loop = 50
 	// test consistensy of putUint48 and uint48
 	for i := 0; i < loop; i++ {

model/flow/identifier.go

@@ -6,7 +6,6 @@ import (
 	"encoding/binary"
 	"encoding/hex"
 	"fmt"
-	"math/rand"
 	"reflect"
 
 	"github.com/ipfs/go-cid"
@@ -16,6 +15,7 @@ import (
 	"github.com/onflow/flow-go/crypto/hash"
 	"github.com/onflow/flow-go/model/fingerprint"
 	"github.com/onflow/flow-go/storage/merkle"
+	"github.com/onflow/flow-go/utils/rand"
 )
 
 const IdentifierLen = 32
@@ -179,21 +179,24 @@ func CheckConcatSum(sum Identifier, fps ...Identifier) bool {
 	return sum == computed
 }
 
-// Sample returns random sample of length 'size' of the ids
-// [Fisher-Yates shuffle](https://en.wikipedia.org/wiki/Fisher-Yates_shuffle).
-func Sample(size uint, ids ...Identifier) []Identifier {
+// Sample returns non-deterministic random sample of length 'size' of the ids
+func Sample(size uint, ids ...Identifier) ([]Identifier, error) {
 	n := uint(len(ids))
 	dup := make([]Identifier, 0, n)
 	dup = append(dup, ids...)
 	// if sample size is greater than total size, return all the elements
 	if n <= size {
-		return dup
+		return dup, nil
 	}
-	for i := uint(0); i < size; i++ {
-		j := uint(rand.Intn(int(n - i)))
-		dup[i], dup[j+i] = dup[j+i], dup[i]
+	swap := func(i, j uint) {
+		dup[i], dup[j] = dup[j], dup[i]
 	}
-	return dup[:size]
+
+	err := rand.Samples(n, size, swap)
+	if err != nil {
+		return nil, fmt.Errorf("generating randoms failed: %w", err)
+	}
+	return dup[:size], nil
 }
 
 func CidToId(c cid.Cid) (Identifier, error) {

model/flow/identifierList.go

@@ -2,7 +2,6 @@ package flow
 
 import (
 	"bytes"
-	"math/rand"
 	"sort"
 
 	"github.com/rs/zerolog/log"
@@ -103,15 +102,8 @@ func (il IdentifierList) Union(other IdentifierList) IdentifierList {
 	return union
 }
 
-// DeterministicSample returns deterministic random sample from the `IdentifierList` using the given seed
-func (il IdentifierList) DeterministicSample(size uint, seed int64) IdentifierList {
-	rand.Seed(seed)
-	return il.Sample(size)
-}
-
 // Sample returns random sample of length 'size' of the ids
-// [Fisher-Yates shuffle](https://en.wikipedia.org/wiki/Fisher–Yates_shuffle).
-func (il IdentifierList) Sample(size uint) IdentifierList {
+func (il IdentifierList) Sample(size uint) (IdentifierList, error) {
 	return Sample(size, il...)
 }
 

model/flow/identifierList_test.go

@@ -5,7 +5,6 @@ import (
 	"math/rand"
 	"sort"
 	"testing"
-	"time"
 
 	"github.com/stretchr/testify/require"
 
@@ -21,7 +20,7 @@ func TestIdentifierListSort(t *testing.T) {
 	var ids flow.IdentifierList = unittest.IdentifierListFixture(count)
 
 	// shuffles array before sorting to enforce some pseudo-randomness
-	rand.Seed(time.Now().UnixNano())
+
 	rand.Shuffle(ids.Len(), ids.Swap)
 
 	sort.Sort(ids)

model/flow/identifier_test.go

@@ -1,10 +1,10 @@
 package flow_test
 
 import (
+	"crypto/rand"
 	"encoding/binary"
 	"encoding/json"
 	"fmt"
-	"math/rand"
 	"testing"
 
 	blocks "github.com/ipfs/go-block-format"
@@ -66,20 +66,23 @@ func TestIdentifierSample(t *testing.T) {
 
 	t.Run("Sample creates a random sample", func(t *testing.T) {
 		sampleSize := uint(5)
-		sample := flow.Sample(sampleSize, ids...)
+		sample, err := flow.Sample(sampleSize, ids...)
+		require.NoError(t, err)
 		require.Len(t, sample, int(sampleSize))
 		require.NotEqual(t, sample, ids[:sampleSize])
 	})
 
 	t.Run("sample size greater than total size results in the original list", func(t *testing.T) {
 		sampleSize := uint(len(ids) + 1)
-		sample := flow.Sample(sampleSize, ids...)
+		sample, err := flow.Sample(sampleSize, ids...)
+		require.NoError(t, err)
 		require.Equal(t, sample, ids)
 	})
 
 	t.Run("sample size of zero results in an empty list", func(t *testing.T) {
 		sampleSize := uint(0)
-		sample := flow.Sample(sampleSize, ids...)
+		sample, err := flow.Sample(sampleSize, ids...)
+		require.NoError(t, err)
 		require.Empty(t, sample)
 	})
 }
@@ -131,7 +134,8 @@ func TestCIDConversion(t *testing.T) {
 
 	// generate random CID
 	data := make([]byte, 4)
-	rand.Read(data)
+	_, err = rand.Read(data)
+	require.NoError(t, err)
 	cid = blocks.NewBlock(data).Cid()
 
 	id, err = flow.CidToId(cid)