Dont process blocks twice (#12905)

* keep track of block being synced

* gazelle

* use maps

* shutup deepsource

* change godoc

* Radek's review

* Do not process block twice if it's already being processed

* add unit test

beacon-chain/blockchain/testing/mock.go

@@ -71,6 +71,7 @@ type ChainService struct {
 	FinalizedRoots              map[[32]byte]bool
 	OptimisticRoots             map[[32]byte]bool
 	BlockSlot                   primitives.Slot
+	SyncingRoot                 [32]byte
 }
 
 func (s *ChainService) Ancestor(ctx context.Context, root []byte, slot primitives.Slot) ([]byte, error) {
@@ -609,6 +610,6 @@ func (s *ChainService) UnrealizedJustifiedPayloadBlockHash() [32]byte {
 func (*ChainService) SendNewBlobEvent(_ [32]byte, _ uint64) {}
 
 // BlockBeingSynced mocks the same method in the chain service
-func (*ChainService) BlockBeingSynced(_ [32]byte) bool {
-	return false
+func (c *ChainService) BlockBeingSynced(root [32]byte) bool {
+	return root == c.SyncingRoot
 }

beacon-chain/sync/pending_blocks_queue.go

@@ -3,6 +3,7 @@ package sync
 import (
 	"context"
 	"encoding/hex"
+	"fmt"
 	"sort"
 	"sync"
 	"time"
@@ -100,6 +101,12 @@ func (s *Service) processPendingBlocks(ctx context.Context) error {
 				span.End()
 				return err
 			}
+			// No need to process the same block if we are already processing it
+			if s.cfg.chain.BlockBeingSynced(blkRoot) {
+				rootString := fmt.Sprintf("%#x", blkRoot)
+				log.WithField("BlockRoot", rootString).Info("Skipping pending block already being processed")
+				continue
+			}
 
 			inDB := s.cfg.beaconDB.HasBlock(ctx, blkRoot)
 			// No need to process the same block twice.
@@ -126,12 +133,12 @@ func (s *Service) processPendingBlocks(ctx context.Context) error {
 				continue
 			}
 
-			parentInDb := s.cfg.beaconDB.HasBlock(ctx, b.Block().ParentRoot())
+			parentRoot := b.Block().ParentRoot()
+			parentInDb := s.cfg.beaconDB.HasBlock(ctx, parentRoot)
 			hasPeer := len(pids) != 0
 
 			// Only request for missing parent block if it's not in beaconDB, not in pending cache
 			// and has peer in the peer list.
-			parentRoot := b.Block().ParentRoot()
 			if !inPendingQueue && !parentInDb && hasPeer {
 				log.WithFields(logrus.Fields{
 					"currentSlot": b.Block().Slot(),

beacon-chain/sync/pending_blocks_queue_test.go

@@ -30,6 +30,7 @@ import (
 	"github.com/prysmaticlabs/prysm/v4/testing/assert"
 	"github.com/prysmaticlabs/prysm/v4/testing/require"
 	"github.com/prysmaticlabs/prysm/v4/testing/util"
+	logTest "github.com/sirupsen/logrus/hooks/test"
 )
 
 //	/- b1 - b2
@@ -846,3 +847,40 @@ func TestService_ProcessBadPendingBlocks(t *testing.T) {
 	// remove with a different block from the same slot.
 	require.NoError(t, r.deleteBlockFromPendingQueue(b.Block.Slot, bB, b1Root))
 }
+
+func TestAlreadySyncingBlock(t *testing.T) {
+	ctx := context.Background()
+	db := dbtest.SetupDB(t)
+	hook := logTest.NewGlobal()
+
+	mockChain := &mock.ChainService{
+		FinalizedCheckPoint: &ethpb.Checkpoint{
+			Epoch: 0,
+		},
+	}
+
+	p1 := p2ptest.NewTestP2P(t)
+	r := &Service{
+		cfg: &config{
+			p2p:      p1,
+			beaconDB: db,
+			chain:    mockChain,
+			clock:    startup.NewClock(time.Unix(0, 0), [32]byte{}),
+			stateGen: stategen.New(db, doublylinkedtree.New()),
+		},
+		slotToPendingBlocks: gcache.New(time.Second, 2*time.Second),
+		seenPendingBlocks:   make(map[[32]byte]bool),
+	}
+	r.initCaches()
+
+	b := util.NewBeaconBlock()
+	b.Block.Slot = 2
+	bRoot, err := b.Block.HashTreeRoot()
+	require.NoError(t, err)
+	wsb, err := blocks.NewSignedBeaconBlock(b)
+	require.NoError(t, err)
+	require.NoError(t, r.insertBlockToPendingQueue(b.Block.Slot, wsb, bRoot))
+	mockChain.SyncingRoot = bRoot
+	require.NoError(t, r.processPendingBlocks(ctx))
+	require.LogsContain(t, hook, "Skipping pending block already being processed")
+}