From 58c0899676e4be7ac3c2fa4deab83cdefbc257d1 Mon Sep 17 00:00:00 2001 From: terencechain Date: Tue, 3 Oct 2023 09:36:23 -0700 Subject: [PATCH] Don't mark block as bad in `validateBeaconBlock` for pending queue (#12983) * Don't mark block as bad in validateBeaconBlock for pending queue * Fix tests --- beacon-chain/sync/pending_blocks_queue.go | 1 - beacon-chain/sync/pending_blocks_queue_test.go | 16 ++++++++-------- beacon-chain/sync/validate_beacon_blocks.go | 1 + 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/beacon-chain/sync/pending_blocks_queue.go b/beacon-chain/sync/pending_blocks_queue.go index f4d8a01ae..259cb5044 100644 --- a/beacon-chain/sync/pending_blocks_queue.go +++ b/beacon-chain/sync/pending_blocks_queue.go @@ -159,7 +159,6 @@ func (s *Service) processPendingBlocks(ctx context.Context) error { case errors.Is(ErrOptimisticParent, err): // Ok to continue process block with parent that is an optimistic candidate. case err != nil: log.WithError(err).WithField("slot", b.Block().Slot()).Debug("Could not validate block") - s.setBadBlock(ctx, blkRoot) tracing.AnnotateError(span, err) span.End() continue diff --git a/beacon-chain/sync/pending_blocks_queue_test.go b/beacon-chain/sync/pending_blocks_queue_test.go index 45fa146de..a21aacb0f 100644 --- a/beacon-chain/sync/pending_blocks_queue_test.go +++ b/beacon-chain/sync/pending_blocks_queue_test.go @@ -108,7 +108,7 @@ func TestRegularSyncBeaconBlockSubscriber_ProcessPendingBlocks1(t *testing.T) { require.NoError(t, r.processPendingBlocks(context.Background())) // Marks a block as bad require.NoError(t, r.processPendingBlocks(context.Background())) // Bad block removed on second run - assert.Equal(t, 1, len(r.slotToPendingBlocks.Items()), "Incorrect size for slot to pending blocks cache") + assert.Equal(t, 2, len(r.slotToPendingBlocks.Items()), "Incorrect size for slot to pending blocks cache") assert.Equal(t, 2, len(r.seenPendingBlocks), "Incorrect size for seen pending block") } @@ -181,7 +181,7 @@ func TestRegularSyncBeaconBlockSubscriber_OptimisticStatus(t *testing.T) { require.NoError(t, r.processPendingBlocks(context.Background())) // Marks a block as bad require.NoError(t, r.processPendingBlocks(context.Background())) // Bad block removed on second run - assert.Equal(t, 1, len(r.slotToPendingBlocks.Items()), "Incorrect size for slot to pending blocks cache") + assert.Equal(t, 2, len(r.slotToPendingBlocks.Items()), "Incorrect size for slot to pending blocks cache") assert.Equal(t, 2, len(r.seenPendingBlocks), "Incorrect size for seen pending block") } @@ -255,9 +255,9 @@ func TestRegularSyncBeaconBlockSubscriber_ExecutionEngineTimesOut(t *testing.T) require.NoError(t, r.processPendingBlocks(context.Background())) // Marks a block as bad require.NoError(t, r.processPendingBlocks(context.Background())) // Bad block removed on second run - assert.Equal(t, 1, len(r.slotToPendingBlocks.Items()), "Incorrect size for slot to pending blocks cache") + assert.Equal(t, 2, len(r.slotToPendingBlocks.Items()), "Incorrect size for slot to pending blocks cache") assert.Equal(t, 2, len(r.seenPendingBlocks), "Incorrect size for seen pending block") - require.Equal(t, 1, len(r.badBlockCache.Keys())) // Account for the bad block above + require.Equal(t, 0, len(r.badBlockCache.Keys())) // Account for the bad block above require.Equal(t, 0, len(r.seenBlockCache.Keys())) } @@ -464,8 +464,8 @@ func TestRegularSyncBeaconBlockSubscriber_ProcessPendingBlocks_2Chains(t *testin require.NoError(t, r.processPendingBlocks(context.Background())) // Marks a block as bad require.NoError(t, r.processPendingBlocks(context.Background())) // Bad block removed on second run - assert.Equal(t, 1, len(r.slotToPendingBlocks.Items()), "Incorrect size for slot to pending blocks cache") - assert.Equal(t, 1, len(r.seenPendingBlocks), "Incorrect size for seen pending block") + assert.Equal(t, 2, len(r.slotToPendingBlocks.Items()), "Incorrect size for slot to pending blocks cache") + assert.Equal(t, 2, len(r.seenPendingBlocks), "Incorrect size for seen pending block") // Add b2 to the cache wsb, err = blocks.NewSignedBeaconBlock(b2) @@ -477,8 +477,8 @@ func TestRegularSyncBeaconBlockSubscriber_ProcessPendingBlocks_2Chains(t *testin require.NoError(t, r.processPendingBlocks(context.Background())) // Marks a block as bad require.NoError(t, r.processPendingBlocks(context.Background())) // Bad block removed on second run - assert.Equal(t, 0, len(r.slotToPendingBlocks.Items()), "Incorrect size for slot to pending blocks cache") - assert.Equal(t, 0, len(r.seenPendingBlocks), "Incorrect size for seen pending block") + assert.Equal(t, 2, len(r.slotToPendingBlocks.Items()), "Incorrect size for slot to pending blocks cache") + assert.Equal(t, 2, len(r.seenPendingBlocks), "Incorrect size for seen pending block") } func TestRegularSyncBeaconBlockSubscriber_PruneOldPendingBlocks(t *testing.T) { diff --git a/beacon-chain/sync/validate_beacon_blocks.go b/beacon-chain/sync/validate_beacon_blocks.go index 88aa64209..bd2246455 100644 --- a/beacon-chain/sync/validate_beacon_blocks.go +++ b/beacon-chain/sync/validate_beacon_blocks.go @@ -229,6 +229,7 @@ func (s *Service) validateBeaconBlock(ctx context.Context, blk interfaces.ReadOn defer span.End() if err := validateDenebBeaconBlock(blk.Block()); err != nil { + s.setBadBlock(ctx, blockRoot) return err }