Revive race detection flag and race conditions fixes in the test exec…

…ution (#1963)

* Provide race and shuffle flags when running unit tests (#1925)

* Provide race and shuffle flags

* Fix race condition in TestBlockchain_VerifyBlockParent test

* Fix race conditions in Test_TxPool_validateTx

* Capture loop variable

* Fix TestAddTxsInOrder

* Linter fixes

* Fix TestBlockchain_VerifyBlockBody race conditions

* Fix Test_VerifySignature_NegativeCases

* Fix TestDropKnownGossipTx

* Retrieve tx hash in a thread safe manner

* Fix TestStatusPubSub race condition

* Fix peers_test

* Fix Test_newTracer

* Provide race flag when building binary and running fuzz tests

* Fix data races in fund and deposit_erc20 commands

* Fix TestEventSubscription_ProcessedEvents

* Fix TestSimpleGossip (hopefully)

* Fix data race in e2e tests (txRelayer.SendTransactionLocal)

* Avoid race conditions in ibft/signer package

* Increase timeout in TestEventSubscription_ProcessedEvents

* Fix TestResetAccounts_Enqueued flakiness

* Remove race flags from e2e and property tests

* Update command/bridge/deposit/erc20/deposit_erc20.go

Co-authored-by: Victor Castell <victor@polygon.technology>

* Fix transaction.Copy function

---------

Co-authored-by: Victor Castell <victor@polygon.technology>

* Remove race detection flag when building a binary

* Minor simplification

* Remove lock from transaction object

* Remove parallel execution of event tracker store tests

* Remove local var when calculating tx hash

* Remove unneded code

* Remove lengthWithLock function from queue account

* Remove parallel execution of TestEventTracker_TrackSyncEvents

---------

Co-authored-by: Victor Castell <victor@polygon.technology>

Makefile

@@ -59,7 +59,7 @@ generate-bsd-licenses: check-git
 
 .PHONY: test
 test: check-go
-	go test -coverprofile coverage.out -timeout 20m `go list ./... | grep -v e2e`
+	go test -race -shuffle=on -coverprofile coverage.out -timeout 20m `go list ./... | grep -v e2e`
 
 .PHONY: fuzz-test
 fuzz-test: check-go

blockchain/blockchain_test.go

@@ -1096,7 +1096,7 @@ func TestBlockchain_VerifyBlockParent(t *testing.T) {
 		// Set up the storage callback
 		storageCallback := func(storage *storage.MockStorage) {
 			storage.HookReadHeader(func(hash types.Hash) (*types.Header, error) {
-				return emptyHeader, nil
+				return emptyHeader.Copy(), nil
 			})
 		}
 
@@ -1110,7 +1110,7 @@ func TestBlockchain_VerifyBlockParent(t *testing.T) {
 		// Create a dummy block whose parent hash will
 		// not match the computed parent hash
 		block := &types.Block{
-			Header: emptyHeader,
+			Header: emptyHeader.Copy(),
 		}
 
 		assert.ErrorIs(t, blockchain.verifyBlockParent(block), ErrParentHashMismatch)
@@ -1122,7 +1122,7 @@ func TestBlockchain_VerifyBlockParent(t *testing.T) {
 		// Set up the storage callback
 		storageCallback := func(storage *storage.MockStorage) {
 			storage.HookReadHeader(func(hash types.Hash) (*types.Header, error) {
-				return emptyHeader, nil
+				return emptyHeader.Copy(), nil
 			})
 		}
 
@@ -1149,7 +1149,7 @@ func TestBlockchain_VerifyBlockParent(t *testing.T) {
 		// Set up the storage callback
 		storageCallback := func(storage *storage.MockStorage) {
 			storage.HookReadHeader(func(hash types.Hash) (*types.Header, error) {
-				return emptyHeader, nil
+				return emptyHeader.Copy(), nil
 			})
 		}
 
@@ -1164,7 +1164,7 @@ func TestBlockchain_VerifyBlockParent(t *testing.T) {
 		block := &types.Block{
 			Header: &types.Header{
 				Number:     10,
-				ParentHash: emptyHeader.Hash,
+				ParentHash: emptyHeader.Copy().Hash,
 			},
 		}
 
@@ -1180,7 +1180,7 @@ func TestBlockchain_VerifyBlockParent(t *testing.T) {
 		// Set up the storage callback
 		storageCallback := func(storage *storage.MockStorage) {
 			storage.HookReadHeader(func(hash types.Hash) (*types.Header, error) {
-				return emptyHeader, nil
+				return emptyHeader.Copy(), nil
 			})
 		}
 
@@ -1286,7 +1286,7 @@ func TestBlockchain_VerifyBlockBody(t *testing.T) {
 		storageCallback := func(storage *storage.MockStorage) {
 			// This is used for parent fetching
 			storage.HookReadHeader(func(hash types.Hash) (*types.Header, error) {
-				return emptyHeader, nil
+				return emptyHeader.Copy(), nil
 			})
 		}
 
@@ -1326,7 +1326,7 @@ func TestBlockchain_VerifyBlockBody(t *testing.T) {
 		storageCallback := func(storage *storage.MockStorage) {
 			// This is used for parent fetching
 			storage.HookReadHeader(func(hash types.Hash) (*types.Header, error) {
-				return emptyHeader, nil
+				return emptyHeader.Copy(), nil
 			})
 		}
 
@@ -1385,12 +1385,13 @@ func TestBlockchain_CalculateBaseFee(t *testing.T) {
 	}
 
 	for i, test := range tests {
+		i := i
 		test := test
 
-		t.Run(fmt.Sprintf("%d", i), func(t *testing.T) {
+		t.Run(fmt.Sprintf("test case #%d", i+1), func(t *testing.T) {
 			t.Parallel()
 
-			blockchain := Blockchain{
+			blockchain := &Blockchain{
 				config: &chain.Chain{
 					Params: &chain.Params{
 						Forks: &chain.Forks{

chain/params.go

@@ -129,6 +129,16 @@ func (f *Forks) At(block uint64) ForksInTime {
 	}
 }
 
+// Copy creates a deep copy of Forks map
+func (f Forks) Copy() *Forks {
+	copiedForks := make(Forks, len(f))
+	for key, value := range f {
+		copiedForks[key] = value.Copy()
+	}
+
+	return &copiedForks
+}
+
 type Fork struct {
 	Block  uint64                  `json:"block"`
 	Params *forkmanager.ForkParams `json:"params,omitempty"`
@@ -142,6 +152,19 @@ func (f Fork) Active(block uint64) bool {
 	return block >= f.Block
 }
 
+// Copy creates a deep copy of Fork
+func (f Fork) Copy() Fork {
+	var fp *forkmanager.ForkParams
+	if f.Params != nil {
+		fp = f.Params.Copy()
+	}
+
+	return Fork{
+		Block:  f.Block,
+		Params: fp,
+	}
+}
+
 // ForksInTime should contain all supported forks by current edge version
 type ForksInTime struct {
 	Homestead,

command/bridge/deposit/erc20/deposit_erc20.go

@@ -195,6 +195,8 @@ func runCommand(cmd *cobra.Command, _ []string) {
 					return fmt.Errorf("failed to create tx input: %w", err)
 				}
 
+				var receipt *ethgo.Receipt
+
 				receipt, err = txRelayer.SendTransaction(depositTxn, depositorKey)
 				if err != nil {
 					return fmt.Errorf("receiver: %s, amount: %s, error: %w", receiver, amount, err)

command/rootchain/fund/fund.go

@@ -120,7 +120,10 @@ func runCommand(cmd *cobra.Command, _ []string) {
 				fundAddr := ethgo.Address(validatorAddr)
 				txn := helper.CreateTransaction(ethgo.ZeroAddress, &fundAddr, nil, params.amountValues[i], true)
 
-				var receipt *ethgo.Receipt
+				var (
+					receipt *ethgo.Receipt
+					err     error
+				)
 
 				if params.deployerPrivateKey != "" {
 					receipt, err = txRelayer.SendTransaction(txn, deployerKey)

consensus/ibft/signer/extra_test.go

@@ -21,8 +21,6 @@ func JSONMarshalHelper(t *testing.T, extra *IstanbulExtra) string {
 }
 
 func TestIstanbulExtraMarshalAndUnmarshal(t *testing.T) {
-	t.Parallel()
-
 	tests := []struct {
 		name  string
 		extra *IstanbulExtra
@@ -99,8 +97,6 @@ func TestIstanbulExtraMarshalAndUnmarshal(t *testing.T) {
 		test := test
 
 		t.Run(test.name, func(t *testing.T) {
-			t.Parallel()
-
 			// create original data
 			originalExtraJSON := JSONMarshalHelper(t, test.extra)
 
@@ -119,8 +115,6 @@ func TestIstanbulExtraMarshalAndUnmarshal(t *testing.T) {
 }
 
 func Test_packProposerSealIntoExtra(t *testing.T) {
-	t.Parallel()
-
 	newProposerSeal := []byte("new proposer seal")
 
 	tests := []struct {
@@ -197,8 +191,6 @@ func Test_packProposerSealIntoExtra(t *testing.T) {
 		test := test
 
 		t.Run(test.name, func(t *testing.T) {
-			t.Parallel()
-
 			originalProposerSeal := test.extra.ProposerSeal
 
 			// create expected data
@@ -233,8 +225,6 @@ func Test_packProposerSealIntoExtra(t *testing.T) {
 }
 
 func Test_packCommittedSealsAndRoundNumberIntoExtra(t *testing.T) {
-	t.Parallel()
-
 	tests := []struct {
 		name              string
 		extra             *IstanbulExtra
@@ -331,8 +321,6 @@ func Test_packCommittedSealsAndRoundNumberIntoExtra(t *testing.T) {
 		test := test
 
 		t.Run(test.name, func(t *testing.T) {
-			t.Parallel()
-
 			originalCommittedSeals := test.extra.CommittedSeals
 
 			// create expected data
@@ -370,8 +358,6 @@ func Test_packCommittedSealsAndRoundNumberIntoExtra(t *testing.T) {
 }
 
 func Test_unmarshalRLPForParentCS(t *testing.T) {
-	t.Parallel()
-
 	tests := []struct {
 		name        string
 		extra       *IstanbulExtra
@@ -423,8 +409,6 @@ func Test_unmarshalRLPForParentCS(t *testing.T) {
 		test := test
 
 		t.Run(test.name, func(t *testing.T) {
-			t.Parallel()
-
 			bytesData := test.extra.MarshalRLPTo(nil)
 
 			assert.NoError(t, test.targetExtra.unmarshalRLPForParentCS(bytesData))
@@ -440,8 +424,6 @@ func Test_unmarshalRLPForParentCS(t *testing.T) {
 }
 
 func Test_putIbftExtra(t *testing.T) {
-	t.Parallel()
-
 	tests := []struct {
 		name   string
 		header *types.Header
@@ -493,8 +475,6 @@ func Test_putIbftExtra(t *testing.T) {
 		test := test
 
 		t.Run(test.name, func(t *testing.T) {
-			t.Parallel()
-
 			putIbftExtra(test.header, test.extra)
 
 			expectedExtraHeader := make([]byte, IstanbulExtraVanity)

consensus/ibft/signer/signer_test.go

@@ -737,8 +737,6 @@ func TestVerifyCommittedSeal(t *testing.T) {
 }
 
 func TestSignerWriteCommittedSeals(t *testing.T) {
-	t.Parallel()
-
 	var round0 uint64 = 0
 
 	tests := []struct {
@@ -849,8 +847,6 @@ func TestSignerWriteCommittedSeals(t *testing.T) {
 		test := test
 
 		t.Run(test.name, func(t *testing.T) {
-			t.Parallel()
-
 			signer := newTestSingleKeyManagerSigner(test.keyManager)
 
 			header, err := signer.WriteCommittedSeals(test.header, test.roundNumber, test.sealMap)

consensus/polybft/fsm.go

@@ -711,7 +711,5 @@ func createStateTransactionWithData(blockNumber uint64, target types.Address, in
 		GasPrice: big.NewInt(0),
 	}
 
-	tx.ComputeHash(blockNumber)
-
-	return tx
+	return tx.ComputeHash(blockNumber)
 }

consensus/polybft/signer/signature_test.go

@@ -47,20 +47,26 @@ func Test_VerifySignature_NegativeCases(t *testing.T) {
 
 	require.True(t, signature.Verify(blsKey.PublicKey(), validTestMsg, DomainValidatorSet))
 
+	rawSig, err := signature.Marshal()
+	require.NoError(t, err)
+
 	t.Run("Wrong public key", func(t *testing.T) {
 		t.Parallel()
 
+		sigTemp, err := UnmarshalSignature(rawSig)
+		require.NoError(t, err)
+
 		for i := 0; i < 100; i++ {
 			x, randomG2, err := bn256.RandomG2(rand.Reader)
 			require.NoError(t, err)
 
 			publicKey := blsKey.PublicKey()
 			publicKey.g2.Add(publicKey.g2, randomG2) // change public key g2 point
-			require.False(t, signature.Verify(publicKey, validTestMsg, DomainValidatorSet))
+			require.False(t, sigTemp.Verify(publicKey, validTestMsg, DomainValidatorSet))
 
 			publicKey = blsKey.PublicKey()
 			publicKey.g2.ScalarMult(publicKey.g2, x) // change public key g2 point
-			require.False(t, signature.Verify(publicKey, validTestMsg, DomainValidatorSet))
+			require.False(t, sigTemp.Verify(publicKey, validTestMsg, DomainValidatorSet))
 		}
 	})
 
@@ -70,11 +76,14 @@ func Test_VerifySignature_NegativeCases(t *testing.T) {
 		msgCopy := make([]byte, len(validTestMsg))
 		copy(msgCopy, validTestMsg)
 
+		sigTemp, err := UnmarshalSignature(rawSig)
+		require.NoError(t, err)
+
 		for i := 0; i < len(msgCopy); i++ {
 			b := msgCopy[i]
 			msgCopy[i] = b + 1
 
-			require.False(t, signature.Verify(blsKey.PublicKey(), msgCopy, DomainValidatorSet))
+			require.False(t, sigTemp.Verify(blsKey.PublicKey(), msgCopy, DomainValidatorSet))
 			msgCopy[i] = b
 		}
 	})
@@ -86,16 +95,13 @@ func Test_VerifySignature_NegativeCases(t *testing.T) {
 			x, randomG1, err := bn256.RandomG1(rand.Reader)
 			require.NoError(t, err)
 
-			raw, err := signature.Marshal()
-			require.NoError(t, err)
-
-			sigCopy, err := UnmarshalSignature(raw)
+			sigCopy, err := UnmarshalSignature(rawSig)
 			require.NoError(t, err)
 
 			sigCopy.g1.Add(sigCopy.g1, randomG1) // change signature
 			require.False(t, sigCopy.Verify(blsKey.PublicKey(), validTestMsg, DomainValidatorSet))
 
-			sigCopy, err = UnmarshalSignature(raw)
+			sigCopy, err = UnmarshalSignature(rawSig)
 			require.NoError(t, err)
 
 			sigCopy.g1.ScalarMult(sigCopy.g1, x) // change signature

forkmanager/fork.go

@@ -40,6 +40,23 @@ type ForkParams struct {
 	BlockTimeDrift *uint64 `json:"blockTimeDrift,omitempty"`
 }
 
+// Copy creates a deep copy of ForkParams
+func (fp *ForkParams) Copy() *ForkParams {
+	maxValSetSize := *fp.MaxValidatorSetSize
+	epochSize := *fp.EpochSize
+	sprintSize := *fp.SprintSize
+	blockTime := *fp.BlockTime
+	blockTimeDrift := *fp.BlockTimeDrift
+
+	return &ForkParams{
+		MaxValidatorSetSize: &maxValSetSize,
+		EpochSize:           &epochSize,
+		SprintSize:          &sprintSize,
+		BlockTime:           &blockTime,
+		BlockTimeDrift:      &blockTimeDrift,
+	}
+}
+
 // forkHandler defines one custom handler
 type forkHandler struct {
 	// Handler should be active from block `FromBlockNumber``

network/e2e_testing.go

@@ -340,9 +340,9 @@ func MeshJoin(servers ...*Server) []error {
 
 	var wg sync.WaitGroup
 
-	for indx := 0; indx < numServers; indx++ {
-		for innerIndx := 0; innerIndx < numServers; innerIndx++ {
-			if innerIndx > indx {
+	for sourceIdx := 0; sourceIdx < numServers; sourceIdx++ {
+		for destIdx := 0; destIdx < numServers; destIdx++ {
+			if destIdx > sourceIdx {
 				wg.Add(1)
 
 				go func(src, dest int) {
@@ -354,9 +354,9 @@ func MeshJoin(servers ...*Server) []error {
 						DefaultBufferTimeout,
 						DefaultJoinTimeout,
 					); joinErr != nil {
-						appendJoinError(fmt.Errorf("unable to join peers, %w", joinErr))
+						appendJoinError(fmt.Errorf("unable to join peers %d -> %d, %w", src, dest, joinErr))
 					}
-				}(indx, innerIndx)
+				}(sourceIdx, destIdx)
 			}
 		}
 	}