Coreth sync nits

.github/workflows/tests.yml

@@ -36,19 +36,29 @@ jobs:
         run: scripts/shellcheck.sh
 
   unit_test:
-    name: Golang Unit Tests
-    runs-on: ubuntu-20.04
+    name: Golang Unit Tests v${{ matrix.go }} (${{ matrix.os }})
+    runs-on: ${{ matrix.os }}
+    strategy:
+      fail-fast: false
+      matrix:
+        os: [macos-latest, ubuntu-20.04, ubuntu-latest, windows-latest]
     steps:
       - uses: actions/checkout@v4
       - uses: actions/setup-go@v5
         with:
           go-version: ${{ env.min_go_version }}
           check-latest: true
+      - name: Set timeout on Windows # Windows UT run slower and need a longer timeout
+        shell: bash
+        if: matrix.os == 'windows-latest'
+        run: echo "TIMEOUT=1200s" >> $GITHUB_ENV
       - run: go mod download
         shell: bash
       - run: ./scripts/build.sh ./build/subnetevm
         shell: bash
-      - run: ./scripts/build_test.sh -race
+      - run: ./scripts/build_test.sh
+        env:
+         TIMEOUT: ${{ env.TIMEOUT }}
         shell: bash
       - run: ./scripts/coverage.sh
         shell: bash

consensus/dummy/consensus.go

@@ -236,13 +236,22 @@ func (self *DummyEngine) verifyHeader(chain consensus.ChainHeaderReader, header
 	}
 	// Verify the existence / non-existence of excessBlobGas
 	cancun := chain.Config().IsCancun(header.Number, header.Time)
-	if !cancun && header.ExcessBlobGas != nil {
-		return fmt.Errorf("invalid excessBlobGas: have %d, expected nil", header.ExcessBlobGas)
-	}
-	if !cancun && header.BlobGasUsed != nil {
-		return fmt.Errorf("invalid blobGasUsed: have %d, expected nil", header.BlobGasUsed)
-	}
-	if cancun {
+	if !cancun {
+		switch {
+		case header.ExcessBlobGas != nil:
+			return fmt.Errorf("invalid excessBlobGas: have %d, expected nil", *header.ExcessBlobGas)
+		case header.BlobGasUsed != nil:
+			return fmt.Errorf("invalid blobGasUsed: have %d, expected nil", *header.BlobGasUsed)
+		case header.ParentBeaconRoot != nil:
+			return fmt.Errorf("invalid parentBeaconRoot, have %#x, expected nil", *header.ParentBeaconRoot)
+		}
+	} else {
+		if header.ParentBeaconRoot == nil {
+			return errors.New("header is missing beaconRoot")
+		}
+		if *header.ParentBeaconRoot != (common.Hash{}) {
+			return fmt.Errorf("invalid parentBeaconRoot, have %#x, expected empty", *header.ParentBeaconRoot)
+		}
 		if err := eip4844.VerifyEIP4844Header(parent, header); err != nil {
 			return err
 		}

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/state_processor_test.go

@@ -472,8 +472,7 @@ func GenerateBadBlock(parent *types.Block, engine consensus.Engine, txs types.Tr
 		header.ExcessBlobGas = &excess
 		header.BlobGasUsed = &used
 
-		beaconRoot := common.HexToHash("0xbeac00")
-		header.ParentBeaconRoot = &beaconRoot
+		header.ParentBeaconRoot = new(common.Hash)
 	}
 	// Assemble and return the final block for sealing
 	return types.NewBlock(header, txs, nil, receipts, trie.NewStackTrie(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())
 			}
 		}
 	}

eth/backend.go

@@ -43,7 +43,6 @@ import (
 	"github.com/ava-labs/subnet-evm/core/rawdb"
 	"github.com/ava-labs/subnet-evm/core/state/pruner"
 	"github.com/ava-labs/subnet-evm/core/txpool"
-	"github.com/ava-labs/subnet-evm/core/txpool/blobpool"
 	"github.com/ava-labs/subnet-evm/core/txpool/legacypool"
 	"github.com/ava-labs/subnet-evm/core/types"
 	"github.com/ava-labs/subnet-evm/core/vm"
@@ -242,12 +241,12 @@ func New(
 
 	eth.bloomIndexer.Start(eth.blockchain)
 
-	config.BlobPool.Datadir = ""
-	blobPool := blobpool.New(config.BlobPool, &chainWithFinalBlock{eth.blockchain})
+	// config.BlobPool.Datadir = ""
+	// blobPool := blobpool.New(config.BlobPool, &chainWithFinalBlock{eth.blockchain})
 
 	legacyPool := legacypool.New(config.TxPool, eth.blockchain)
 
-	eth.txPool, err = txpool.New(new(big.Int).SetUint64(config.TxPool.PriceLimit), eth.blockchain, []txpool.SubPool{legacyPool, blobPool})
+	eth.txPool, err = txpool.New(new(big.Int).SetUint64(config.TxPool.PriceLimit), eth.blockchain, []txpool.SubPool{legacyPool}) //, blobPool})
 	if err != nil {
 		return nil, err
 	}

eth/chain_with_final_block.go

@@ -1,3 +1,4 @@
+//nolint:unused
 package eth
 
 import (

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))
 	}

miner/worker.go

@@ -102,7 +102,7 @@ type worker struct {
 	mu         sync.RWMutex   // The lock used to protect the coinbase and extra fields
 	coinbase   common.Address
 	clock      *mockable.Clock // Allows us mock the clock for testing
-	beaconRoot *common.Hash    // TODO: not set anywhere, retained for upstream compatibility and future use
+	beaconRoot *common.Hash    // TODO: set to empty hash, retained for upstream compatibility and future use
 }
 
 func newWorker(config *Config, chainConfig *params.ChainConfig, engine consensus.Engine, eth Backend, mux *event.TypeMux, clock *mockable.Clock) *worker {
@@ -115,6 +115,7 @@ func newWorker(config *Config, chainConfig *params.ChainConfig, engine consensus
 		mux:         mux,
 		coinbase:    config.Etherbase,
 		clock:       clock,
+		beaconRoot:  &common.Hash{},
 	}
 
 	return worker

plugin/evm/block_verification.go

@@ -144,16 +144,29 @@ func (v blockValidator) SyntacticVerify(b *Block, rules params.Rules) error {
 	// Verify the existence / non-existence of excessBlobGas
 	cancun := rules.IsCancun
 	if !cancun && ethHeader.ExcessBlobGas != nil {
-		return fmt.Errorf("invalid excessBlobGas: have %d, expected nil", ethHeader.ExcessBlobGas)
+		return fmt.Errorf("invalid excessBlobGas: have %d, expected nil", *ethHeader.ExcessBlobGas)
 	}
 	if !cancun && ethHeader.BlobGasUsed != nil {
-		return fmt.Errorf("invalid blobGasUsed: have %d, expected nil", ethHeader.BlobGasUsed)
+		return fmt.Errorf("invalid blobGasUsed: have %d, expected nil", *ethHeader.BlobGasUsed)
 	}
 	if cancun && ethHeader.ExcessBlobGas == nil {
 		return errors.New("header is missing excessBlobGas")
 	}
 	if cancun && ethHeader.BlobGasUsed == nil {
 		return errors.New("header is missing blobGasUsed")
 	}
+	if !cancun && ethHeader.ParentBeaconRoot != nil {
+		return fmt.Errorf("invalid parentBeaconRoot: have %x, expected nil", *ethHeader.ParentBeaconRoot)
+	}
+	// TODO: decide what to do after Cancun
+	// currently we are enforcing it to be empty hash
+	if cancun {
+		switch {
+		case ethHeader.ParentBeaconRoot == nil:
+			return errors.New("header is missing parentBeaconRoot")
+		case *ethHeader.ParentBeaconRoot != (common.Hash{}):
+			return fmt.Errorf("invalid parentBeaconRoot: have %x, expected empty hash", ethHeader.ParentBeaconRoot)
+		}
+	}
 	return nil
 }

plugin/evm/gossip.go

@@ -7,6 +7,7 @@ import (
 	"context"
 	"fmt"
 	"sync"
+	"sync/atomic"
 	"time"
 
 	"github.com/ava-labs/avalanchego/ids"
@@ -108,10 +109,28 @@ 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)
+	if sub == nil {
+		log.Warn("failed to subscribe to new txs event")
+		return
+	}
+	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",
 	)
 }