add matrix and race to UTs (#530)

* add matrix and race to UTs

* remove unnecessary runners

* clean script

* lazy read bonus blocks

* increase timeout

* increase frequency

* apply fix fow windows

* increase timeout

* disable fail fast

* increase frequency and timeout

* use latest

* fix coma

* better log

* fix require

* Fix formatting

* fix eventually formats

* test out new wg

* Revert "test out new wg"

This reverts commit 5d4a0a8a17bf78751163e948a6a65dd241b057aa.

* check tx indexes after each block accept

* mark as flaky

* disable blobpool

* add IsSubscribed for testing

* remove subscribed when return

* use atomic bool

* attempt to fix flaky indexing test

core/blockchain_test.go

@@ -692,40 +692,49 @@ func TestTransactionSkipIndexing(t *testing.T) {
 
 	// test1: Init block chain and check all indices has been skipped.
 	chainDB := rawdb.NewMemoryDatabase()
-	chain, err := createAndInsertChain(chainDB, conf, gspec, blocks, common.Hash{})
+	chain, err := createAndInsertChain(chainDB, conf, gspec, blocks, common.Hash{},
+		func(b *types.Block) {
+			bNumber := b.NumberU64()
+			checkTxIndicesHelper(t, nil, bNumber+1, bNumber+1, bNumber, chainDB, false) // check all indices has been skipped
+		})
 	require.NoError(err)
-	currentBlockNumber := chain.CurrentBlock().Number.Uint64()
-	checkTxIndicesHelper(t, nil, currentBlockNumber+1, currentBlockNumber+1, currentBlockNumber, chainDB, false) // check all indices has been skipped
 	chain.Stop()
 
 	// test2: specify lookuplimit with tx index skipping enabled. Blocks should not be indexed but tail should be updated.
 	conf.TxLookupLimit = 2
-	chain, err = createAndInsertChain(chainDB, conf, gspec, blocks2[0:1], chain.CurrentHeader().Hash())
+	chainDB = rawdb.NewMemoryDatabase()
+	chain, err = createAndInsertChain(chainDB, conf, gspec, blocks, common.Hash{},
+		func(b *types.Block) {
+			bNumber := b.NumberU64()
+			tail := bNumber - conf.TxLookupLimit + 1
+			checkTxIndicesHelper(t, &tail, bNumber+1, bNumber+1, bNumber, chainDB, false) // check all indices has been skipped
+		})
 	require.NoError(err)
-	currentBlockNumber = chain.CurrentBlock().Number.Uint64()
-	tail := currentBlockNumber - conf.TxLookupLimit + 1
-	checkTxIndicesHelper(t, &tail, currentBlockNumber+1, currentBlockNumber+1, currentBlockNumber, chainDB, false) // check all indices has been skipped
 	chain.Stop()
 
 	// test3: tx index skipping and unindexer disabled. Blocks should be indexed and tail should be updated.
 	conf.TxLookupLimit = 0
 	conf.SkipTxIndexing = false
 	chainDB = rawdb.NewMemoryDatabase()
-	chain, err = createAndInsertChain(chainDB, conf, gspec, blocks, common.Hash{})
+	chain, err = createAndInsertChain(chainDB, conf, gspec, blocks, common.Hash{},
+		func(b *types.Block) {
+			bNumber := b.NumberU64()
+			checkTxIndicesHelper(t, nil, 0, bNumber, bNumber, chainDB, false) // check all indices has been indexed
+		})
 	require.NoError(err)
-	currentBlockNumber = chain.CurrentBlock().Number.Uint64()
-	checkTxIndicesHelper(t, nil, 0, currentBlockNumber, currentBlockNumber, chainDB, false) // check all indices has been indexed
 	chain.Stop()
 
 	// now change tx index skipping to true and check that the indices are skipped for the last block
 	// and old indices are removed up to the tail, but [tail, current) indices are still there.
 	conf.TxLookupLimit = 2
 	conf.SkipTxIndexing = true
-	chain, err = createAndInsertChain(chainDB, conf, gspec, blocks2[0:1], chain.CurrentHeader().Hash())
+	chain, err = createAndInsertChain(chainDB, conf, gspec, blocks2[0:1], chain.CurrentHeader().Hash(),
+		func(b *types.Block) {
+			bNumber := b.NumberU64()
+			tail := bNumber - conf.TxLookupLimit + 1
+			checkTxIndicesHelper(t, &tail, tail, bNumber-1, bNumber, chainDB, false)
+		})
 	require.NoError(err)
-	currentBlockNumber = chain.CurrentBlock().Number.Uint64()
-	tail = currentBlockNumber - conf.TxLookupLimit + 1
-	checkTxIndicesHelper(t, &tail, tail, currentBlockNumber-1, currentBlockNumber, chainDB, false)
 	chain.Stop()
 }
 
@@ -1302,7 +1311,7 @@ func TestEIP3651(t *testing.T) {
 	}
 }
 
-func createAndInsertChain(db ethdb.Database, cacheConfig *CacheConfig, gspec *Genesis, blocks types.Blocks, lastAcceptedHash common.Hash) (*BlockChain, error) {
+func createAndInsertChain(db ethdb.Database, cacheConfig *CacheConfig, gspec *Genesis, blocks types.Blocks, lastAcceptedHash common.Hash, accepted func(*types.Block)) (*BlockChain, error) {
 	chain, err := createBlockChain(db, cacheConfig, gspec, lastAcceptedHash)
 	if err != nil {
 		return nil, err
@@ -1316,8 +1325,11 @@ func createAndInsertChain(db ethdb.Database, cacheConfig *CacheConfig, gspec *Ge
 		if err != nil {
 			return nil, err
 		}
+		chain.DrainAcceptorQueue()
+		if accepted != nil {
+			accepted(block)
+		}
 	}
-	chain.DrainAcceptorQueue()
 
 	return chain, nil
 }

core/test_blockchain.go

@@ -1665,18 +1665,18 @@ func CheckTxIndices(t *testing.T, expectedTail *uint64, head uint64, db ethdb.Da
 // [indexedTo] is the block number to which the transactions should be indexed.
 // [head] is the block number of the head block.
 func checkTxIndicesHelper(t *testing.T, expectedTail *uint64, indexedFrom uint64, indexedTo uint64, head uint64, db ethdb.Database, allowNilBlocks bool) {
-	require := require.New(t)
 	if expectedTail == nil {
-		require.Nil(rawdb.ReadTxIndexTail(db))
+		require.Nil(t, rawdb.ReadTxIndexTail(db))
 	} else {
 		var stored uint64
 		tailValue := *expectedTail
-		require.Eventually(
-			func() bool {
+
+		require.EventuallyWithTf(t,
+			func(c *assert.CollectT) {
 				stored = *rawdb.ReadTxIndexTail(db)
-				return tailValue == stored
+				require.Equalf(t, tailValue, stored, "expected tail to be %d, found %d", tailValue, stored)
 			},
-			10*time.Second, 100*time.Millisecond, "expected tail to be %d eventually (was %d)", tailValue, stored)
+			30*time.Second, 500*time.Millisecond, "expected tail to be %d eventually", tailValue)
 	}
 
 	for i := uint64(0); i <= head; i++ {
@@ -1687,9 +1687,9 @@ func checkTxIndicesHelper(t *testing.T, expectedTail *uint64, indexedFrom uint64
 		for _, tx := range block.Transactions() {
 			index := rawdb.ReadTxLookupEntry(db, tx.Hash())
 			if i < indexedFrom {
-				require.Nilf(index, "Transaction indices should be deleted, number %d hash %s", i, tx.Hash().Hex())
+				require.Nilf(t, index, "Transaction indices should be deleted, number %d hash %s", i, tx.Hash().Hex())
 			} else if i <= indexedTo {
-				require.NotNilf(index, "Missing transaction indices, number %d hash %s", i, tx.Hash().Hex())
+				require.NotNilf(t, index, "Missing transaction indices, number %d hash %s", i, tx.Hash().Hex())
 			}
 		}
 	}

metrics/opentsdb_test.go

@@ -46,7 +46,8 @@ func TestExampleOpenTSB(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
-	if have, want := w.String(), string(wantB); have != want {
+	want := strings.ReplaceAll(string(wantB), "\r\n", "\n")
+	if have := w.String(); have != want {
 		t.Errorf("\nhave:\n%v\nwant:\n%v\n", have, want)
 		t.Logf("have vs want:\n%v", findFirstDiffPos(have, want))
 	}

plugin/evm/gossip.go

@@ -7,6 +7,7 @@ import (
 	"context"
 	"fmt"
 	"sync"
+	"sync/atomic"
 	"time"
 
 	"github.com/ava-labs/avalanchego/ids"
@@ -108,10 +109,24 @@ type GossipEthTxPool struct {
 
 	bloom *gossip.BloomFilter
 	lock  sync.RWMutex
+
+	// subscribed is set to true when the gossip subscription is active
+	// mostly used for testing
+	subscribed atomic.Bool
+}
+
+// IsSubscribed returns whether or not the gossip subscription is active.
+func (g *GossipEthTxPool) IsSubscribed() bool {
+	return g.subscribed.Load()
 }
 
 func (g *GossipEthTxPool) Subscribe(ctx context.Context) {
-	g.mempool.SubscribeNewTxsEvent(g.pendingTxs)
+	sub := g.mempool.SubscribeNewTxsEvent(g.pendingTxs)
+	g.subscribed.CompareAndSwap(false, true)
+	defer func() {
+		sub.Unsubscribe()
+		g.subscribed.CompareAndSwap(true, false)
+	}()
 
 	for {
 		select {

plugin/evm/gossip_test.go

@@ -21,6 +21,7 @@ import (
 	"github.com/ethereum/go-ethereum/common"
 	"github.com/ethereum/go-ethereum/crypto"
 	"github.com/prometheus/client_golang/prometheus"
+	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
 
@@ -61,6 +62,10 @@ func TestGossipSubscribe(t *testing.T) {
 	defer cancel()
 	go gossipTxPool.Subscribe(ctx)
 
+	require.Eventually(func() bool {
+		return gossipTxPool.IsSubscribed()
+	}, 10*time.Second, 500*time.Millisecond, "expected gossipTxPool to be subscribed")
+
 	// create eth txs
 	ethTxs := getValidEthTxs(key, 10, big.NewInt(226*params.GWei))
 
@@ -70,20 +75,17 @@ func TestGossipSubscribe(t *testing.T) {
 		require.NoError(err, "failed adding subnet-evm tx to remote mempool")
 	}
 
-	require.Eventually(
-		func() bool {
+	require.EventuallyWithTf(
+		func(c *assert.CollectT) {
 			gossipTxPool.lock.RLock()
 			defer gossipTxPool.lock.RUnlock()
 
-			for _, tx := range ethTxs {
-				if !gossipTxPool.bloom.Has(&GossipEthTx{Tx: tx}) {
-					return false
-				}
+			for i, tx := range ethTxs {
+				require.Truef(gossipTxPool.bloom.Has(&GossipEthTx{Tx: tx}), "expected tx[%d] to be in bloom filter", i)
 			}
-			return true
 		},
-		10*time.Second,
-		10*time.Millisecond,
+		30*time.Second,
+		500*time.Millisecond,
 		"expected all transactions to eventually be in the bloom filter",
 	)
 }

plugin/evm/syncervm_test.go

@@ -293,7 +293,7 @@ func createSyncServerAndClientVMs(t *testing.T, test syncTest, numBlocks int) *s
 		signedTx, err := types.SignTx(tx, types.NewEIP155Signer(serverVM.chainConfig.ChainID), testKeys[0])
 		require.NoError(err)
 		gen.AddTx(signedTx)
-	})
+	}, nil)
 
 	// make some accounts
 	trieDB := trie.NewDatabase(serverVM.chaindb, nil)
@@ -449,12 +449,19 @@ func testSyncerVM(t *testing.T, vmSetup *syncVMSetup, test syncTest) {
 
 	lastNumber := syncerVM.blockChain.LastAcceptedBlock().NumberU64()
 	// check the last block is indexed
-	block := rawdb.ReadBlock(syncerVM.chaindb, rawdb.ReadCanonicalHash(syncerVM.chaindb, lastNumber), lastNumber)
-	for _, tx := range block.Transactions() {
+	lastSyncedBlock := rawdb.ReadBlock(syncerVM.chaindb, rawdb.ReadCanonicalHash(syncerVM.chaindb, lastNumber), lastNumber)
+	for _, tx := range lastSyncedBlock.Transactions() {
 		index := rawdb.ReadTxLookupEntry(syncerVM.chaindb, tx.Hash())
 		require.NotNilf(index, "Miss transaction indices, number %d hash %s", lastNumber, tx.Hash().Hex())
 	}
 
+	// tail should be the last block synced
+	if syncerVM.ethConfig.TxLookupLimit != 0 {
+		tail := lastSyncedBlock.NumberU64()
+
+		core.CheckTxIndices(t, &tail, tail, syncerVM.chaindb, true)
+	}
+
 	blocksToBuild := 10
 	txsPerBlock := 10
 	toAddress := testEthAddrs[1] // arbitrary choice
@@ -475,15 +482,18 @@ func testSyncerVM(t *testing.T, vmSetup *syncVMSetup, test syncTest) {
 				break
 			}
 		}
-	})
-
-	if syncerVM.ethConfig.TxLookupLimit != 0 {
-		tail := syncerVM.blockChain.LastAcceptedBlock().NumberU64() - syncerVM.ethConfig.TxLookupLimit + 1
-
-		syncerVM.blockChain.DrainAcceptorQueue()
-
-		core.CheckTxIndices(t, &tail, syncerVM.blockChain.LastAcceptedBlock().NumberU64(), syncerVM.chaindb, true)
-	}
+	},
+		func(block *types.Block) {
+			if syncerVM.ethConfig.TxLookupLimit != 0 {
+				tail := block.NumberU64() - syncerVM.ethConfig.TxLookupLimit + 1
+				// tail should be the minimum last synced block, since we skipped it to the last block
+				if tail < lastSyncedBlock.NumberU64() {
+					tail = lastSyncedBlock.NumberU64()
+				}
+				core.CheckTxIndices(t, &tail, block.NumberU64(), syncerVM.chaindb, true)
+			}
+		},
+	)
 
 	// check we can transition to [NormalOp] state and continue to process blocks.
 	require.NoError(syncerVM.SetState(context.Background(), snow.NormalOp))
@@ -505,7 +515,18 @@ func testSyncerVM(t *testing.T, vmSetup *syncVMSetup, test syncTest) {
 				break
 			}
 		}
-	})
+	},
+		func(block *types.Block) {
+			if syncerVM.ethConfig.TxLookupLimit != 0 {
+				tail := block.NumberU64() - syncerVM.ethConfig.TxLookupLimit + 1
+				// tail should be the minimum last synced block, since we skipped it to the last block
+				if tail < lastSyncedBlock.NumberU64() {
+					tail = lastSyncedBlock.NumberU64()
+				}
+				core.CheckTxIndices(t, &tail, block.NumberU64(), syncerVM.chaindb, true)
+			}
+		},
+	)
 }
 
 // patchBlock returns a copy of [blk] with [root] and updates [db] to
@@ -528,7 +549,7 @@ func patchBlock(blk *types.Block, root common.Hash, db ethdb.Database) *types.Bl
 // generateAndAcceptBlocks uses [core.GenerateChain] to generate blocks, then
 // calls Verify and Accept on each generated block
 // TODO: consider using this helper function in vm_test.go and elsewhere in this package to clean up tests
-func generateAndAcceptBlocks(t *testing.T, vm *VM, numBlocks int, gen func(int, *core.BlockGen)) {
+func generateAndAcceptBlocks(t *testing.T, vm *VM, numBlocks int, gen func(int, *core.BlockGen), accepted func(*types.Block)) {
 	t.Helper()
 
 	// acceptExternalBlock defines a function to parse, verify, and accept a block once it has been
@@ -548,6 +569,10 @@ func generateAndAcceptBlocks(t *testing.T, vm *VM, numBlocks int, gen func(int,
 		if err := vmBlock.Accept(context.Background()); err != nil {
 			t.Fatal(err)
 		}
+
+		if accepted != nil {
+			accepted(block)
+		}
 	}
 	_, _, err := core.GenerateChain(
 		vm.chainConfig,

scripts/build_test.sh

@@ -3,9 +3,6 @@
 set -o errexit
 set -o nounset
 set -o pipefail
-
-export GOGC=25
-
 # TODO(marun) Ensure the working directory is the repository root or a non-canonical set of tests may be executed
 
 # Root directory
@@ -24,4 +21,4 @@ source "$SUBNET_EVM_PATH"/scripts/constants.sh
 # parallelism, and test coverage.
 # DO NOT RUN tests from the top level "tests" directory since they are run by ginkgo
 # shellcheck disable=SC2046
-go test -shuffle=on -race -coverprofile=coverage.out -covermode=atomic -timeout="30m" "$@" $(go list ./... | grep -v github.com/ava-labs/subnet-evm/tests)
+go test -shuffle=on -race -timeout="${TIMEOUT:-600s}" -coverprofile=coverage.out -covermode=atomic  "$@" $(go list ./... | grep -v github.com/ava-labs/subnet-evm/tests)

scripts/lint.sh

@@ -8,7 +8,7 @@ set -o pipefail
 # checking for this specific version is an attempt to avoid skew
 # between local and CI execution. The latest version (v1.55.1) seems
 # to cause spurious failures
-KNOWN_GOOD_VERSION="v1.54"
+KNOWN_GOOD_VERSION="v1.56"
 VERSION="$(golangci-lint --version | sed -e 's+golangci-lint has version \(v1.*\)\..* built.*+\1+')"
 if [[ "${VERSION}" != "${KNOWN_GOOD_VERSION}" ]]; then
   echo "expected golangci-lint ${KNOWN_GOOD_VERSION}, but ${VERSION} was used"