Merge branch 'ethereum:master' into gethintegration

Author: GrapeBaBa

cmd/devp2p/internal/ethtest/suite.go

@@ -70,6 +70,7 @@ func (s *Suite) EthTests() []utesting.Test {
 		{Name: "Status", Fn: s.TestStatus},
 		// get block headers
 		{Name: "GetBlockHeaders", Fn: s.TestGetBlockHeaders},
+		{Name: "GetNonexistentBlockHeaders", Fn: s.TestGetNonexistentBlockHeaders},
 		{Name: "SimultaneousRequests", Fn: s.TestSimultaneousRequests},
 		{Name: "SameRequestID", Fn: s.TestSameRequestID},
 		{Name: "ZeroRequestID", Fn: s.TestZeroRequestID},
@@ -158,6 +159,48 @@ func (s *Suite) TestGetBlockHeaders(t *utesting.T) {
 	}
 }
 
+func (s *Suite) TestGetNonexistentBlockHeaders(t *utesting.T) {
+	t.Log(`This test sends GetBlockHeaders requests for nonexistent blocks (using max uint64 value) 
+to check if the node disconnects after receiving multiple invalid requests.`)
+
+	conn, err := s.dial()
+	if err != nil {
+		t.Fatalf("dial failed: %v", err)
+	}
+	defer conn.Close()
+
+	if err := conn.peer(s.chain, nil); err != nil {
+		t.Fatalf("peering failed: %v", err)
+	}
+
+	// Create request with max uint64 value for a nonexistent block
+	badReq := &eth.GetBlockHeadersPacket{
+		GetBlockHeadersRequest: &eth.GetBlockHeadersRequest{
+			Origin:  eth.HashOrNumber{Number: ^uint64(0)},
+			Amount:  1,
+			Skip:    0,
+			Reverse: false,
+		},
+	}
+
+	// Send request 10 times. Some clients are lient on the first few invalids.
+	for i := 0; i < 10; i++ {
+		badReq.RequestId = uint64(i)
+		if err := conn.Write(ethProto, eth.GetBlockHeadersMsg, badReq); err != nil {
+			if err == errDisc {
+				t.Fatalf("peer disconnected after %d requests", i+1)
+			}
+			t.Fatalf("write failed: %v", err)
+		}
+	}
+
+	// Check if peer disconnects at the end.
+	code, _, err := conn.Read()
+	if err == errDisc || code == discMsg {
+		t.Fatal("peer improperly disconnected")
+	}
+}
+
 func (s *Suite) TestSimultaneousRequests(t *utesting.T) {
 	t.Log(`This test requests blocks headers from the node, performing two requests
 concurrently, with different request IDs.`)

cmd/geth/consolecmd_test.go

@@ -39,9 +39,8 @@ const (
 // child g gets a temporary data directory.
 func runMinimalGeth(t *testing.T, args ...string) *testgeth {
 	// --holesky to make the 'writing genesis to disk' faster (no accounts)
-	// --networkid=1337 to avoid cache bump
 	// --syncmode=full to avoid allocating fast sync bloom
-	allArgs := []string{"--holesky", "--networkid", "1337", "--authrpc.port", "0", "--syncmode=full", "--port", "0",
+	allArgs := []string{"--holesky", "--authrpc.port", "0", "--syncmode=full", "--port", "0",
 		"--nat", "none", "--nodiscover", "--maxpeers", "0", "--cache", "64",
 		"--datadir.minfreedisk", "0"}
 	return runGeth(t, append(allArgs, args...)...)

cmd/utils/flags.go

@@ -1571,8 +1571,8 @@ func setRequiredBlocks(ctx *cli.Context, cfg *ethconfig.Config) {
 
 // SetEthConfig applies eth-related command line flags to the config.
 func SetEthConfig(ctx *cli.Context, stack *node.Node, cfg *ethconfig.Config) {
-	// Avoid conflicting network flags
-	flags.CheckExclusive(ctx, MainnetFlag, DeveloperFlag, SepoliaFlag, HoleskyFlag, HoodiFlag)
+	// Avoid conflicting network flags, don't allow network id override on preset networks
+	flags.CheckExclusive(ctx, MainnetFlag, DeveloperFlag, SepoliaFlag, HoleskyFlag, HoodiFlag, NetworkIdFlag)
 	flags.CheckExclusive(ctx, DeveloperFlag, ExternalSignerFlag) // Can't use both ephemeral unlocked and external signer
 
 	// Set configurations from CLI flags
@@ -1743,33 +1743,23 @@ func SetEthConfig(ctx *cli.Context, stack *node.Node, cfg *ethconfig.Config) {
 	// Override any default configs for hard coded networks.
 	switch {
 	case ctx.Bool(MainnetFlag.Name):
-		if !ctx.IsSet(NetworkIdFlag.Name) {
-			cfg.NetworkId = 1
-		}
+		cfg.NetworkId = 1
 		cfg.Genesis = core.DefaultGenesisBlock()
 		SetDNSDiscoveryDefaults(cfg, params.MainnetGenesisHash)
 	case ctx.Bool(HoleskyFlag.Name):
-		if !ctx.IsSet(NetworkIdFlag.Name) {
-			cfg.NetworkId = 17000
-		}
+		cfg.NetworkId = 17000
 		cfg.Genesis = core.DefaultHoleskyGenesisBlock()
 		SetDNSDiscoveryDefaults(cfg, params.HoleskyGenesisHash)
 	case ctx.Bool(SepoliaFlag.Name):
-		if !ctx.IsSet(NetworkIdFlag.Name) {
-			cfg.NetworkId = 11155111
-		}
+		cfg.NetworkId = 11155111
 		cfg.Genesis = core.DefaultSepoliaGenesisBlock()
 		SetDNSDiscoveryDefaults(cfg, params.SepoliaGenesisHash)
 	case ctx.Bool(HoodiFlag.Name):
-		if !ctx.IsSet(NetworkIdFlag.Name) {
-			cfg.NetworkId = 560048
-		}
+		cfg.NetworkId = 560048
 		cfg.Genesis = core.DefaultHoodiGenesisBlock()
 		SetDNSDiscoveryDefaults(cfg, params.HoodiGenesisHash)
 	case ctx.Bool(DeveloperFlag.Name):
-		if !ctx.IsSet(NetworkIdFlag.Name) {
-			cfg.NetworkId = 1337
-		}
+		cfg.NetworkId = 1337
 		cfg.SyncMode = ethconfig.FullSync
 		// Create new developer account or reuse existing one
 		var (

core/blockchain_reader.go

@@ -270,42 +270,20 @@ func (bc *BlockChain) GetAncestor(hash common.Hash, number, ancestor uint64, max
 // GetTransactionLookup retrieves the lookup along with the transaction
 // itself associate with the given transaction hash.
 //
-// An error will be returned if the transaction is not found, and background
-// indexing for transactions is still in progress. The transaction might be
-// reachable shortly once it's indexed.
-//
-// A null will be returned in the transaction is not found and background
-// transaction indexing is already finished. The transaction is not existent
-// from the node's perspective.
-func (bc *BlockChain) GetTransactionLookup(hash common.Hash) (*rawdb.LegacyTxLookupEntry, *types.Transaction, error) {
+// A null will be returned if the transaction is not found. This can be due to
+// the transaction indexer not being finished. The caller must explicitly check
+// the indexer progress.
+func (bc *BlockChain) GetTransactionLookup(hash common.Hash) (*rawdb.LegacyTxLookupEntry, *types.Transaction) {
 	bc.txLookupLock.RLock()
 	defer bc.txLookupLock.RUnlock()
 
 	// Short circuit if the txlookup already in the cache, retrieve otherwise
 	if item, exist := bc.txLookupCache.Get(hash); exist {
-		return item.lookup, item.transaction, nil
+		return item.lookup, item.transaction
 	}
 	tx, blockHash, blockNumber, txIndex := rawdb.ReadTransaction(bc.db, hash)
 	if tx == nil {
-		progress, err := bc.TxIndexProgress()
-		if err != nil {
-			// No error is returned if the transaction indexing progress is unreachable
-			// due to unexpected internal errors. In such cases, it is impossible to
-			// determine whether the transaction does not exist or has simply not been
-			// indexed yet without a progress marker.
-			//
-			// In such scenarios, the transaction is treated as unreachable, though
-			// this is clearly an unintended and unexpected situation.
-			return nil, nil, nil
-		}
-		// The transaction indexing is not finished yet, returning an
-		// error to explicitly indicate it.
-		if !progress.Done() {
-			return nil, nil, errors.New("transaction indexing still in progress")
-		}
-		// The transaction is already indexed, the transaction is either
-		// not existent or not in the range of index, returning null.
-		return nil, nil, nil
+		return nil, nil
 	}
 	lookup := &rawdb.LegacyTxLookupEntry{
 		BlockHash:  blockHash,
@@ -316,7 +294,23 @@ func (bc *BlockChain) GetTransactionLookup(hash common.Hash) (*rawdb.LegacyTxLoo
 		lookup:      lookup,
 		transaction: tx,
 	})
-	return lookup, tx, nil
+	return lookup, tx
+}
+
+// TxIndexDone returns true if the transaction indexer has finished indexing.
+func (bc *BlockChain) TxIndexDone() bool {
+	progress, err := bc.TxIndexProgress()
+	if err != nil {
+		// No error is returned if the transaction indexing progress is unreachable
+		// due to unexpected internal errors. In such cases, it is impossible to
+		// determine whether the transaction does not exist or has simply not been
+		// indexed yet without a progress marker.
+		//
+		// In such scenarios, the transaction is treated as unreachable, though
+		// this is clearly an unintended and unexpected situation.
+		return true
+	}
+	return progress.Done()
 }
 
 // HasState checks if state trie is fully present in the database or not.
@@ -412,7 +406,7 @@ func (bc *BlockChain) TxIndexProgress() (TxIndexProgress, error) {
 	if bc.txIndexer == nil {
 		return TxIndexProgress{}, errors.New("tx indexer is not enabled")
 	}
-	return bc.txIndexer.txIndexProgress()
+	return bc.txIndexer.txIndexProgress(), nil
 }
 
 // HistoryPruningCutoff returns the configured history pruning point.

core/filtermaps/chain_view.go

@@ -135,14 +135,6 @@ func (cv *ChainView) SharedRange(cv2 *ChainView) common.Range[uint64] {
 	return common.NewRange(0, sharedLen)
 }
 
-// limitedView returns a new chain view that is a truncated version of the parent view.
-func (cv *ChainView) limitedView(newHead uint64) *ChainView {
-	if newHead >= cv.headNumber {
-		return cv
-	}
-	return NewChainView(cv.chain, newHead, cv.BlockHash(newHead))
-}
-
 // equalViews returns true if the two chain views are equivalent.
 func equalViews(cv1, cv2 *ChainView) bool {
 	if cv1 == nil || cv2 == nil {

core/filtermaps/filtermaps.go

@@ -50,7 +50,7 @@ var (
 )
 
 const (
-	databaseVersion       = 1    // reindexed if database version does not match
+	databaseVersion       = 2    // reindexed if database version does not match
 	cachedLastBlocks      = 1000 // last block of map pointers
 	cachedLvPointers      = 1000 // first log value pointer of block pointers
 	cachedBaseRows        = 100  // groups of base layer filter row data
@@ -244,6 +244,8 @@ func NewFilterMaps(db ethdb.KeyValueStore, initView *ChainView, historyCutoff, f
 		disabledCh:        make(chan struct{}),
 		exportFileName:    config.ExportFileName,
 		Params:            params,
+		targetView:        initView,
+		indexedView:       initView,
 		indexedRange: filterMapsRange{
 			initialized:      initialized,
 			headIndexed:      rs.HeadIndexed,
@@ -265,16 +267,8 @@ func NewFilterMaps(db ethdb.KeyValueStore, initView *ChainView, historyCutoff, f
 		baseRowsCache:   lru.NewCache[uint64, [][]uint32](cachedBaseRows),
 		renderSnapshots: lru.NewCache[uint64, *renderedMap](cachedRenderSnapshots),
 	}
+	f.checkRevertRange() // revert maps that are inconsistent with the current chain view
 
-	// Set initial indexer target.
-	f.targetView = initView
-	if f.indexedRange.initialized {
-		f.indexedView = f.initChainView(f.targetView)
-		f.indexedRange.headIndexed = f.indexedRange.blocks.AfterLast() == f.indexedView.HeadNumber()+1
-		if !f.indexedRange.headIndexed {
-			f.indexedRange.headDelimiter = 0
-		}
-	}
 	if f.indexedRange.hasIndexedBlocks() {
 		log.Info("Initialized log indexer",
 			"first block", f.indexedRange.blocks.First(), "last block", f.indexedRange.blocks.Last(),
@@ -303,29 +297,40 @@ func (f *FilterMaps) Stop() {
 	f.closeWg.Wait()
 }
 
-// initChainView returns a chain view consistent with both the current target
-// view and the current state of the log index as found in the database, based
-// on the last block of stored maps.
-// Note that the returned view might be shorter than the existing index if
-// the latest maps are not consistent with targetView.
-func (f *FilterMaps) initChainView(chainView *ChainView) *ChainView {
-	mapIndex := f.indexedRange.maps.AfterLast()
-	for {
-		var ok bool
-		mapIndex, ok = f.lastMapBoundaryBefore(mapIndex)
-		if !ok {
-			break
+// checkRevertRange checks whether the existing index is consistent with the
+// current indexed view and reverts inconsistent maps if necessary.
+func (f *FilterMaps) checkRevertRange() {
+	if f.indexedRange.maps.Count() == 0 {
+		return
+	}
+	lastMap := f.indexedRange.maps.Last()
+	lastBlockNumber, lastBlockId, err := f.getLastBlockOfMap(lastMap)
+	if err != nil {
+		log.Error("Error initializing log index database; resetting log index", "error", err)
+		f.reset()
+		return
+	}
+	for lastBlockNumber > f.indexedView.HeadNumber() || f.indexedView.BlockId(lastBlockNumber) != lastBlockId {
+		// revert last map
+		if f.indexedRange.maps.Count() == 1 {
+			f.reset() // reset database if no rendered maps remained
+			return
 		}
-		lastBlockNumber, lastBlockId, err := f.getLastBlockOfMap(mapIndex)
+		lastMap--
+		newRange := f.indexedRange
+		newRange.maps.SetLast(lastMap)
+		lastBlockNumber, lastBlockId, err = f.getLastBlockOfMap(lastMap)
 		if err != nil {
-			log.Error("Could not initialize indexed chain view", "error", err)
-			break
-		}
-		if lastBlockNumber <= chainView.HeadNumber() && chainView.BlockId(lastBlockNumber) == lastBlockId {
-			return chainView.limitedView(lastBlockNumber)
+			log.Error("Error initializing log index database; resetting log index", "error", err)
+			f.reset()
+			return
 		}
+		newRange.blocks.SetAfterLast(lastBlockNumber) // lastBlockNumber is probably partially indexed
+		newRange.headIndexed = false
+		newRange.headDelimiter = 0
+		// only shorten range and leave map data; next head render will overwrite it
+		f.setRange(f.db, f.indexedView, newRange, false)
 	}
-	return chainView.limitedView(0)
 }
 
 // reset un-initializes the FilterMaps structure and removes all related data from
@@ -662,15 +667,11 @@ func (f *FilterMaps) mapRowIndex(mapIndex, rowIndex uint32) uint64 {
 }
 
 // getBlockLvPointer returns the starting log value index where the log values
-// generated by the given block are located. If blockNumber is beyond the current
-// head then the first unoccupied log value index is returned.
+// generated by the given block are located.
 //
 // Note that this function assumes that the indexer read lock is being held when
 // called from outside the indexerLoop goroutine.
 func (f *FilterMaps) getBlockLvPointer(blockNumber uint64) (uint64, error) {
-	if blockNumber >= f.indexedRange.blocks.AfterLast() && f.indexedRange.headIndexed {
-		return f.indexedRange.headDelimiter + 1, nil
-	}
 	if lvPointer, ok := f.lvPointerCache.Get(blockNumber); ok {
 		return lvPointer, nil
 	}

core/filtermaps/indexer.go

@@ -254,7 +254,7 @@ func (f *FilterMaps) tryIndexHead() error {
 			((!f.loggedHeadIndex && time.Since(f.startedHeadIndexAt) > headLogDelay) ||
 				time.Since(f.lastLogHeadIndex) > logFrequency) {
 			log.Info("Log index head rendering in progress",
-				"first block", f.indexedRange.blocks.First(), "last block", f.indexedRange.blocks.Last(),
+				"firstblock", f.indexedRange.blocks.First(), "lastblock", f.indexedRange.blocks.Last(),
 				"processed", f.indexedRange.blocks.AfterLast()-f.ptrHeadIndex,
 				"remaining", f.indexedView.HeadNumber()-f.indexedRange.blocks.Last(),
 				"elapsed", common.PrettyDuration(time.Since(f.startedHeadIndexAt)))
@@ -266,7 +266,7 @@ func (f *FilterMaps) tryIndexHead() error {
 	}
 	if f.loggedHeadIndex && f.indexedRange.hasIndexedBlocks() {
 		log.Info("Log index head rendering finished",
-			"first block", f.indexedRange.blocks.First(), "last block", f.indexedRange.blocks.Last(),
+			"firstblock", f.indexedRange.blocks.First(), "lastblock", f.indexedRange.blocks.Last(),
 			"processed", f.indexedRange.blocks.AfterLast()-f.ptrHeadIndex,
 			"elapsed", common.PrettyDuration(time.Since(f.startedHeadIndexAt)))
 	}
@@ -323,7 +323,7 @@ func (f *FilterMaps) tryIndexTail() (bool, error) {
 			if f.indexedRange.hasIndexedBlocks() && f.ptrTailIndex >= f.indexedRange.blocks.First() &&
 				(!f.loggedTailIndex || time.Since(f.lastLogTailIndex) > logFrequency) {
 				log.Info("Log index tail rendering in progress",
-					"first block", f.indexedRange.blocks.First(), "last block", f.indexedRange.blocks.Last(),
+					"firstblock", f.indexedRange.blocks.First(), "last block", f.indexedRange.blocks.Last(),
 					"processed", f.ptrTailIndex-f.indexedRange.blocks.First()+tpb,
 					"remaining", remaining,
 					"next tail epoch percentage", f.indexedRange.tailPartialEpoch*100/f.mapsPerEpoch,
@@ -346,7 +346,7 @@ func (f *FilterMaps) tryIndexTail() (bool, error) {
 	}
 	if f.loggedTailIndex && f.indexedRange.hasIndexedBlocks() {
 		log.Info("Log index tail rendering finished",
-			"first block", f.indexedRange.blocks.First(), "last block", f.indexedRange.blocks.Last(),
+			"firstblock", f.indexedRange.blocks.First(), "lastblock", f.indexedRange.blocks.Last(),
 			"processed", f.ptrTailIndex-f.indexedRange.blocks.First(),
 			"elapsed", common.PrettyDuration(time.Since(f.startedTailIndexAt)))
 		f.loggedTailIndex = false

core/filtermaps/map_renderer.go

@@ -468,15 +468,25 @@ func (r *mapRenderer) writeFinishedMaps(pauseCb func() bool) error {
 			r.f.filterMapCache.Remove(mapIndex)
 		}
 	}
+	var blockNumber uint64
+	if r.finished.First() > 0 {
+		// in order to always ensure continuous block pointers, initialize
+		// blockNumber based on the last block of the previous map, then verify
+		// against the first block associated with each rendered map
+		lastBlock, _, err := r.f.getLastBlockOfMap(r.finished.First() - 1)
+		if err != nil {
+			return fmt.Errorf("failed to get last block of previous map %d: %v", r.finished.First()-1, err)
+		}
+		blockNumber = lastBlock + 1
+	}
 	// add or update block pointers
-	blockNumber := r.finishedMaps[r.finished.First()].firstBlock()
 	for mapIndex := range r.finished.Iter() {
 		renderedMap := r.finishedMaps[mapIndex]
-		r.f.storeLastBlockOfMap(batch, mapIndex, renderedMap.lastBlock, renderedMap.lastBlockId)
-		checkWriteCnt()
 		if blockNumber != renderedMap.firstBlock() {
-			panic("non-continuous block numbers")
+			return fmt.Errorf("non-continuous block numbers in rendered map %d (next expected: %d  first rendered: %d)", mapIndex, blockNumber, renderedMap.firstBlock())
 		}
+		r.f.storeLastBlockOfMap(batch, mapIndex, renderedMap.lastBlock, renderedMap.lastBlockId)
+		checkWriteCnt()
 		for _, lvPtr := range renderedMap.blockLvPtrs {
 			r.f.storeBlockLvPointer(batch, blockNumber, lvPtr)
 			checkWriteCnt()