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 <[email protected]>

* Fix transaction.Copy function

---------

Co-authored-by: Victor Castell <[email protected]>

Makefile

@@ -42,7 +42,7 @@ build: check-go check-git
 	$(eval COMMIT_HASH = $(shell git rev-parse HEAD))
 	$(eval BRANCH = $(shell git rev-parse --abbrev-ref HEAD | tr -d '\040\011\012\015\n'))
 	$(eval TIME = $(shell date))
-	go build -o polygon-edge -ldflags="\
+	go build -race -o polygon-edge -ldflags="\
     	-X 'github.com/0xPolygon/polygon-edge/versioning.Version=$(LATEST_VERSION)' \
 		-X 'github.com/0xPolygon/polygon-edge/versioning.Commit=$(COMMIT_HASH)'\
 		-X 'github.com/0xPolygon/polygon-edge/versioning.Branch=$(BRANCH)'\
@@ -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.go

@@ -1002,7 +1002,7 @@ func (b *Blockchain) writeBody(batchWriter *storage.BatchWriter, block *types.Bl
 
 	// Write txn lookups (txHash -> block)
 	for _, txn := range block.Transactions {
-		batchWriter.PutTxLookup(txn.Hash, block.Hash())
+		batchWriter.PutTxLookup(txn.GetHash(), block.Hash())
 	}
 
 	return nil
@@ -1046,7 +1046,7 @@ func (b *Blockchain) recoverFromFieldsInTransactions(transactions []*types.Trans
 
 		sender, err := b.txSigner.Sender(tx)
 		if err != nil {
-			b.logger.Warn("failed to recover from address in Tx", "hash", tx.Hash, "err", err)
+			b.logger.Warn("failed to recover from address in Tx", "hash", tx.GetHash(), "err", err)
 
 			continue
 		}

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

@@ -132,6 +132,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"`
@@ -145,6 +155,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/block_builder.go

@@ -140,7 +140,7 @@ func (b *BlockBuilder) Build(handler func(h *types.Header)) (*types.FullBlock, e
 // WriteTx applies given transaction to the state. If transaction apply fails, it reverts the saved snapshot.
 func (b *BlockBuilder) WriteTx(tx *types.Transaction) error {
 	if tx.Gas > b.params.GasLimit {
-		b.params.Logger.Info("Transaction gas limit exceedes block gas limit", "hash", tx.Hash,
+		b.params.Logger.Info("Transaction gas limit exceedes block gas limit", "hash", tx.GetHash(),
 			"tx gas limit", tx.Gas, "block gas limt", b.params.GasLimit)
 
 		return txpool.ErrBlockLimitExceeded
@@ -171,7 +171,7 @@ write:
 			// execute transactions one by one
 			finished, err := b.writeTxPoolTransaction(tx)
 			if err != nil {
-				b.params.Logger.Debug("Fill transaction error", "hash", tx.Hash, "err", err)
+				b.params.Logger.Debug("Fill transaction error", "hash", tx.GetHash(), "err", err)
 			}
 
 			if finished {

consensus/polybft/fsm.go

@@ -426,24 +426,26 @@ func (f *fsm) VerifyStateTransactions(transactions []*types.Transaction) error {
 			continue
 		}
 
+		txHash := tx.GetHash()
+
 		decodedStateTx, err := decodeStateTransaction(tx.Input)
 		if err != nil {
-			return fmt.Errorf("unknown state transaction: tx = %v, err = %w", tx.Hash, err)
+			return fmt.Errorf("unknown state transaction: tx = %v, err = %w", txHash, err)
 		}
 
 		switch stateTxData := decodedStateTx.(type) {
 		case *CommitmentMessageSigned:
 			if !f.isEndOfSprint {
-				return fmt.Errorf("found commitment tx in block which should not contain it (tx hash=%s)", tx.Hash)
+				return fmt.Errorf("found commitment tx in block which should not contain it (tx hash=%s)", txHash)
 			}
 
 			if commitmentTxExists {
-				return fmt.Errorf("only one commitment tx is allowed per block (tx hash=%s)", tx.Hash)
+				return fmt.Errorf("only one commitment tx is allowed per block (tx hash=%s)", txHash)
 			}
 
 			commitmentTxExists = true
 
-			if err = verifyBridgeCommitmentTx(f.Height(), tx.Hash, stateTxData, f.validators); err != nil {
+			if err = verifyBridgeCommitmentTx(f.Height(), txHash, stateTxData, f.validators); err != nil {
 				return err
 			}
 		case *contractsapi.CommitEpochValidatorSetFn:

consensus/polybft/polybft.go

@@ -749,21 +749,23 @@ func (p *Polybft) PreCommitState(block *types.Block, _ *state.Transition) error
 			continue
 		}
 
+		txHash := tx.GetHash()
+
 		decodedStateTx, err := decodeStateTransaction(tx.Input)
 		if err != nil {
-			return fmt.Errorf("unknown state transaction: tx=%v, error: %w", tx.Hash, err)
+			return fmt.Errorf("unknown state transaction: tx=%v, error: %w", txHash, err)
 		}
 
 		if signedCommitment, ok := decodedStateTx.(*CommitmentMessageSigned); ok {
 			if commitmentTxExists {
-				return fmt.Errorf("only one commitment state tx is allowed per block: %v", tx.Hash)
+				return fmt.Errorf("only one commitment state tx is allowed per block: %v", txHash)
 			}
 
 			commitmentTxExists = true
 
 			if err := verifyBridgeCommitmentTx(
 				block.Number(),
-				tx.Hash,
+				txHash,
 				signedCommitment,
 				validator.NewValidatorSet(validators, p.logger)); err != nil {
 				return err