diff --git a/beacon-chain/blockchain/error.go b/beacon-chain/blockchain/error.go index 7e64ccc346..0a8acba141 100644 --- a/beacon-chain/blockchain/error.go +++ b/beacon-chain/blockchain/error.go @@ -17,3 +17,35 @@ var ( // block is not a valid optimistic candidate block errNotOptimisticCandidate = errors.New("block is not suitable for optimistic sync") ) + +// An invalid block is the block that fails state transition based on the core protocol rules. +// The beacon node shall not be accepting nor building blocks that branch off from an invalid block. +// Some examples of invalid blocks are: +// The block violates state transition rules. +// The block is deemed invalid according to execution layer client. +// The block violates certain fork choice rules (before finalized slot, not finalized ancestor) +type invalidBlock struct { + error +} + +type invalidBlockError interface { + Error() string + InvalidBlock() bool +} + +// InvalidBlock returns true for `invalidBlock`. +func (e invalidBlock) InvalidBlock() bool { + return true +} + +// IsInvalidBlock returns true if the error has `invalidBlock`. +func IsInvalidBlock(e error) bool { + if e == nil { + return false + } + d, ok := e.(invalidBlockError) + if !ok { + return IsInvalidBlock(errors.Unwrap(e)) + } + return d.InvalidBlock() +} diff --git a/beacon-chain/blockchain/error_test.go b/beacon-chain/blockchain/error_test.go new file mode 100644 index 0000000000..89b3ad6514 --- /dev/null +++ b/beacon-chain/blockchain/error_test.go @@ -0,0 +1,17 @@ +package blockchain + +import ( + "testing" + + "github.com/pkg/errors" + "github.com/prysmaticlabs/prysm/testing/require" +) + +func TestIsInvalidBlock(t *testing.T) { + require.Equal(t, false, IsInvalidBlock(ErrInvalidPayload)) + err := invalidBlock{ErrInvalidPayload} + require.Equal(t, true, IsInvalidBlock(err)) + + newErr := errors.Wrap(err, "wrap me") + require.Equal(t, true, IsInvalidBlock(newErr)) +} diff --git a/beacon-chain/blockchain/execution_engine.go b/beacon-chain/blockchain/execution_engine.go index 4eef5a6624..9603c95a7e 100644 --- a/beacon-chain/blockchain/execution_engine.go +++ b/beacon-chain/blockchain/execution_engine.go @@ -174,14 +174,14 @@ func (s *Service) notifyNewPayload(ctx context.Context, postStateVersion int, body := blk.Block().Body() enabled, err := blocks.IsExecutionEnabledUsingHeader(postStateHeader, body) if err != nil { - return false, errors.Wrap(err, "could not determine if execution is enabled") + return false, errors.Wrap(invalidBlock{err}, "could not determine if execution is enabled") } if !enabled { return true, nil } payload, err := body.ExecutionPayload() if err != nil { - return false, errors.Wrap(err, "could not get execution payload") + return false, errors.Wrap(invalidBlock{err}, "could not get execution payload") } lastValidHash, err := s.cfg.ExecutionEngineCaller.NewPayload(ctx, payload) switch err { @@ -213,7 +213,7 @@ func (s *Service) notifyNewPayload(ctx context.Context, postStateVersion int, "blockRoot": fmt.Sprintf("%#x", root), "invalidCount": len(invalidRoots), }).Warn("Pruned invalid blocks") - return false, ErrInvalidPayload + return false, invalidBlock{ErrInvalidPayload} default: return false, errors.WithMessage(ErrUndefinedExecutionEngineError, err.Error()) } diff --git a/beacon-chain/blockchain/execution_engine_test.go b/beacon-chain/blockchain/execution_engine_test.go index 31934eabfa..52b4d6e58e 100644 --- a/beacon-chain/blockchain/execution_engine_test.go +++ b/beacon-chain/blockchain/execution_engine_test.go @@ -387,12 +387,13 @@ func Test_NotifyNewPayload(t *testing.T) { require.NoError(t, fcs.InsertOptimisticBlock(ctx, 1, r, [32]byte{}, params.BeaconConfig().ZeroHash, 0, 0)) tests := []struct { - name string postState state.BeaconState + invalidBlock bool isValidPayload bool blk interfaces.SignedBeaconBlock newPayloadErr error errString string + name string }{ { name: "phase 0 post state", @@ -424,6 +425,7 @@ func Test_NotifyNewPayload(t *testing.T) { newPayloadErr: powchain.ErrInvalidPayloadStatus, errString: ErrInvalidPayload.Error(), isValidPayload: false, + invalidBlock: true, }, { name: "altair pre state, altair block", @@ -535,9 +537,13 @@ func Test_NotifyNewPayload(t *testing.T) { isValidPayload, err := service.notifyNewPayload(ctx, postVersion, postHeader, tt.blk) if tt.errString != "" { require.ErrorContains(t, tt.errString, err) + if tt.invalidBlock { + require.Equal(t, true, IsInvalidBlock(err)) + } } else { require.NoError(t, err) require.Equal(t, tt.isValidPayload, isValidPayload) + require.Equal(t, false, IsInvalidBlock(err)) } }) } @@ -690,7 +696,11 @@ func Test_IsOptimisticCandidateBlock(t *testing.T) { require.NoError(t, service.cfg.BeaconDB.SaveBlock(ctx, wrappedParentBlock)) err = service.optimisticCandidateBlock(ctx, tt.blk) - require.Equal(t, tt.err, err) + if tt.err != nil { + require.Equal(t, tt.err.Error(), err.Error()) + } else { + require.NoError(t, err) + } } } diff --git a/beacon-chain/blockchain/pow_block.go b/beacon-chain/blockchain/pow_block.go index 14a46c3c22..42c305228f 100644 --- a/beacon-chain/blockchain/pow_block.go +++ b/beacon-chain/blockchain/pow_block.go @@ -64,8 +64,9 @@ func (s *Service) validateMergeBlock(ctx context.Context, b interfaces.SignedBea return err } if !valid { - return fmt.Errorf("invalid TTD, configTTD: %s, currentTTD: %s, parentTTD: %s", + err := fmt.Errorf("invalid TTD, configTTD: %s, currentTTD: %s, parentTTD: %s", params.BeaconConfig().TerminalTotalDifficulty, mergeBlockTD, mergeBlockParentTD) + return invalidBlock{err} } log.WithFields(logrus.Fields{ diff --git a/beacon-chain/blockchain/pow_block_test.go b/beacon-chain/blockchain/pow_block_test.go index bb1b5c9e30..d00ccf1def 100644 --- a/beacon-chain/blockchain/pow_block_test.go +++ b/beacon-chain/blockchain/pow_block_test.go @@ -144,7 +144,9 @@ func Test_validateMergeBlock(t *testing.T) { cfg.TerminalTotalDifficulty = "1" params.OverrideBeaconConfig(cfg) - require.ErrorContains(t, "invalid TTD, configTTD: 1, currentTTD: 2, parentTTD: 1", service.validateMergeBlock(ctx, b)) + err = service.validateMergeBlock(ctx, b) + require.ErrorContains(t, "invalid TTD, configTTD: 1, currentTTD: 2, parentTTD: 1", err) + require.Equal(t, true, IsInvalidBlock(err)) } func Test_getBlkParentHashAndTD(t *testing.T) { diff --git a/beacon-chain/blockchain/process_block.go b/beacon-chain/blockchain/process_block.go index 7afccf2fc9..205cf46697 100644 --- a/beacon-chain/blockchain/process_block.go +++ b/beacon-chain/blockchain/process_block.go @@ -93,7 +93,7 @@ func (s *Service) onBlock(ctx context.Context, signed interfaces.SignedBeaconBlo ctx, span := trace.StartSpan(ctx, "blockChain.onBlock") defer span.End() if err := helpers.BeaconBlockIsNil(signed); err != nil { - return err + return invalidBlock{err} } b := signed.Block() @@ -108,7 +108,7 @@ func (s *Service) onBlock(ctx context.Context, signed interfaces.SignedBeaconBlo } postState, err := transition.ExecuteStateTransition(ctx, preState, signed) if err != nil { - return err + return invalidBlock{err} } postStateVersion, postStateHeader, err := getStateVersionAndPayload(postState) if err != nil { @@ -328,7 +328,7 @@ func (s *Service) onBlockBatch(ctx context.Context, blks []interfaces.SignedBeac } if err := helpers.BeaconBlockIsNil(blks[0]); err != nil { - return nil, nil, err + return nil, nil, invalidBlock{err} } b := blks[0].Block() @@ -371,7 +371,7 @@ func (s *Service) onBlockBatch(ctx context.Context, blks []interfaces.SignedBeac set, preState, err = transition.ExecuteStateTransitionNoVerifyAnySig(ctx, preState, b) if err != nil { - return nil, nil, err + return nil, nil, invalidBlock{err} } // Save potential boundary states. if slots.IsEpochStart(preState.Slot()) { @@ -392,7 +392,7 @@ func (s *Service) onBlockBatch(ctx context.Context, blks []interfaces.SignedBeac } verify, err := sigSet.Verify() if err != nil { - return nil, nil, err + return nil, nil, invalidBlock{err} } if !verify { return nil, nil, errors.New("batch block signature verification failed") @@ -664,7 +664,7 @@ func (s *Service) validateMergeTransitionBlock(ctx context.Context, stateVersion // Skip validation if block has an empty payload. payload, err := blk.Block().Body().ExecutionPayload() if err != nil { - return err + return invalidBlock{err} } if blocks.IsEmptyPayload(payload) { return nil diff --git a/beacon-chain/blockchain/process_block_helpers.go b/beacon-chain/blockchain/process_block_helpers.go index e03dca2cd7..1cc1a7dcb3 100644 --- a/beacon-chain/blockchain/process_block_helpers.go +++ b/beacon-chain/blockchain/process_block_helpers.go @@ -115,7 +115,7 @@ func (s *Service) VerifyFinalizedBlkDescendant(ctx context.Context, root [32]byt bytesutil.Trunc(root[:]), finalizedBlk.Slot(), bytesutil.Trunc(bFinalizedRoot), bytesutil.Trunc(fRoot[:])) tracing.AnnotateError(span, err) - return err + return invalidBlock{err} } return nil } @@ -132,7 +132,8 @@ func (s *Service) verifyBlkFinalizedSlot(b interfaces.BeaconBlock) error { return err } if finalizedSlot >= b.Slot() { - return fmt.Errorf("block is equal or earlier than finalized block, slot %d < slot %d", b.Slot(), finalizedSlot) + err = fmt.Errorf("block is equal or earlier than finalized block, slot %d < slot %d", b.Slot(), finalizedSlot) + return invalidBlock{err} } return nil } diff --git a/beacon-chain/blockchain/process_block_test.go b/beacon-chain/blockchain/process_block_test.go index e139687eef..a806bcd289 100644 --- a/beacon-chain/blockchain/process_block_test.go +++ b/beacon-chain/blockchain/process_block_test.go @@ -1318,9 +1318,10 @@ func TestVerifyBlkDescendant(t *testing.T) { finalizedRoot [32]byte } tests := []struct { - name string - args args - wantedErr string + name string + args args + wantedErr string + invalidBlockRoot bool }{ { name: "could not get finalized block in block service cache", @@ -1343,7 +1344,8 @@ func TestVerifyBlkDescendant(t *testing.T) { finalizedRoot: r1, parentRoot: r, }, - wantedErr: "is not a descendant of the current finalized block slot", + wantedErr: "is not a descendant of the current finalized block slot", + invalidBlockRoot: true, }, { name: "is descendant", @@ -1360,6 +1362,9 @@ func TestVerifyBlkDescendant(t *testing.T) { err = service.VerifyFinalizedBlkDescendant(ctx, tt.args.parentRoot) if tt.wantedErr != "" { assert.ErrorContains(t, tt.wantedErr, err) + if tt.invalidBlockRoot { + require.Equal(t, true, IsInvalidBlock(err)) + } } else if err != nil { assert.NoError(t, err) } @@ -1471,6 +1476,60 @@ func TestOnBlock_CanFinalize(t *testing.T) { require.Equal(t, f.Epoch, service.FinalizedCheckpt().Epoch) } +func TestOnBlock_NilBlock(t *testing.T) { + ctx := context.Background() + beaconDB := testDB.SetupDB(t) + fcs := protoarray.New(0, 0, [32]byte{'a'}) + depositCache, err := depositcache.New() + require.NoError(t, err) + opts := []Option{ + WithDatabase(beaconDB), + WithStateGen(stategen.New(beaconDB)), + WithForkChoiceStore(fcs), + WithDepositCache(depositCache), + } + service, err := NewService(ctx, opts...) + require.NoError(t, err) + + err = service.onBlock(ctx, nil, [32]byte{}) + require.Equal(t, true, IsInvalidBlock(err)) +} + +func TestOnBlock_InvalidSignature(t *testing.T) { + ctx := context.Background() + beaconDB := testDB.SetupDB(t) + fcs := protoarray.New(0, 0, [32]byte{'a'}) + depositCache, err := depositcache.New() + require.NoError(t, err) + opts := []Option{ + WithDatabase(beaconDB), + WithStateGen(stategen.New(beaconDB)), + WithForkChoiceStore(fcs), + WithDepositCache(depositCache), + WithStateNotifier(&mock.MockStateNotifier{}), + } + service, err := NewService(ctx, opts...) + require.NoError(t, err) + + gs, keys := util.DeterministicGenesisState(t, 32) + require.NoError(t, service.saveGenesisData(ctx, gs)) + gBlk, err := service.cfg.BeaconDB.GenesisBlock(ctx) + require.NoError(t, err) + gRoot, err := gBlk.Block().HashTreeRoot() + require.NoError(t, err) + service.store.SetFinalizedCheckptAndPayloadHash(ðpb.Checkpoint{Root: gRoot[:]}, [32]byte{}) + + blk, err := util.GenerateFullBlock(gs, keys, util.DefaultBlockGenConfig(), 1) + require.NoError(t, err) + blk.Signature = []byte{'a'} // Mutate the signature. + r, err := blk.Block.HashTreeRoot() + require.NoError(t, err) + wsb, err := wrapper.WrappedSignedBeaconBlock(blk) + require.NoError(t, err) + err = service.onBlock(ctx, wsb, r) + require.Equal(t, true, IsInvalidBlock(err)) +} + func TestOnBlock_CallNewPayloadAndForkchoiceUpdated(t *testing.T) { params.SetupTestConfigCleanup(t) config := params.BeaconConfig() @@ -1972,3 +2031,23 @@ func TestOnBlock_ProcessBlocksParallel(t *testing.T) { service.cfg.ForkChoiceStore = protoarray.New(0, 0, [32]byte{'a'}) } } + +func Test_verifyBlkFinalizedSlot_invalidBlock(t *testing.T) { + ctx := context.Background() + beaconDB := testDB.SetupDB(t) + + fcs := protoarray.New(0, 0, [32]byte{'a'}) + opts := []Option{ + WithDatabase(beaconDB), + WithStateGen(stategen.New(beaconDB)), + WithForkChoiceStore(fcs), + } + service, err := NewService(ctx, opts...) + require.NoError(t, err) + service.store.SetFinalizedCheckptAndPayloadHash(ðpb.Checkpoint{Epoch: 1}, [32]byte{'a'}) + blk := util.HydrateBeaconBlock(ðpb.BeaconBlock{Slot: 1}) + wb, err := wrapper.WrappedBeaconBlock(blk) + require.NoError(t, err) + err = service.verifyBlkFinalizedSlot(wb) + require.Equal(t, true, IsInvalidBlock(err)) +} diff --git a/beacon-chain/sync/BUILD.bazel b/beacon-chain/sync/BUILD.bazel index e6149f3741..6b435ccaa3 100644 --- a/beacon-chain/sync/BUILD.bazel +++ b/beacon-chain/sync/BUILD.bazel @@ -76,7 +76,6 @@ go_library( "//beacon-chain/p2p/encoder:go_default_library", "//beacon-chain/p2p/peers:go_default_library", "//beacon-chain/p2p/types:go_default_library", - "//beacon-chain/powchain:go_default_library", "//beacon-chain/state:go_default_library", "//beacon-chain/state/stategen:go_default_library", "//cache/lru:go_default_library", diff --git a/beacon-chain/sync/pending_blocks_queue.go b/beacon-chain/sync/pending_blocks_queue.go index 6d0b6dc3f6..9e4f797b5b 100644 --- a/beacon-chain/sync/pending_blocks_queue.go +++ b/beacon-chain/sync/pending_blocks_queue.go @@ -12,7 +12,6 @@ import ( "github.com/prysmaticlabs/prysm/beacon-chain/blockchain" "github.com/prysmaticlabs/prysm/beacon-chain/core/helpers" p2ptypes "github.com/prysmaticlabs/prysm/beacon-chain/p2p/types" - "github.com/prysmaticlabs/prysm/beacon-chain/powchain" "github.com/prysmaticlabs/prysm/config/params" "github.com/prysmaticlabs/prysm/consensus-types/interfaces" types "github.com/prysmaticlabs/prysm/consensus-types/primitives" @@ -156,10 +155,6 @@ func (s *Service) processPendingBlocks(ctx context.Context) error { err = s.validateBeaconBlock(ctx, b, blkRoot) switch { case errors.Is(ErrOptimisticParent, err): // Ok to continue process block with parent that is an optimistic candidate. - case errors.Is(blockchain.ErrUndefinedExecutionEngineError, err): - // don't mark the block as bad with an undefined EE error. - log.Debugf("Could not validate block due to undefined ee error %d: %v", b.Block().Slot(), err) - continue case err != nil: log.Debugf("Could not validate block from slot %d: %v", b.Block().Slot(), err) s.setBadBlock(ctx, blkRoot) @@ -170,11 +165,12 @@ func (s *Service) processPendingBlocks(ctx context.Context) error { } if err := s.cfg.chain.ReceiveBlock(ctx, b, blkRoot); err != nil { - if !errors.Is(err, powchain.ErrHTTPTimeout) { - log.Debugf("Could not process block from slot %d: %v", b.Block().Slot(), err) + if blockchain.IsInvalidBlock(err) { tracing.AnnotateError(span, err) s.setBadBlock(ctx, blkRoot) } + log.Debugf("Could not process block from slot %d: %v", b.Block().Slot(), err) + // In the next iteration of the queue, this block will be removed from // the pending queue as it has been marked as a 'bad' block. span.End() diff --git a/beacon-chain/sync/subscriber_beacon_blocks.go b/beacon-chain/sync/subscriber_beacon_blocks.go index 410b79d803..89973ff4b1 100644 --- a/beacon-chain/sync/subscriber_beacon_blocks.go +++ b/beacon-chain/sync/subscriber_beacon_blocks.go @@ -3,11 +3,9 @@ package sync import ( "context" - "github.com/pkg/errors" "github.com/prysmaticlabs/prysm/beacon-chain/blockchain" "github.com/prysmaticlabs/prysm/beacon-chain/core/helpers" "github.com/prysmaticlabs/prysm/beacon-chain/core/transition/interop" - "github.com/prysmaticlabs/prysm/beacon-chain/powchain" "github.com/prysmaticlabs/prysm/config/features" "github.com/prysmaticlabs/prysm/consensus-types/wrapper" ethpb "github.com/prysmaticlabs/prysm/proto/prysm/v1alpha1" @@ -33,7 +31,7 @@ func (s *Service) beaconBlockSubscriber(ctx context.Context, msg proto.Message) } if err := s.cfg.chain.ReceiveBlock(ctx, signed, root); err != nil { - if !errors.Is(err, powchain.ErrHTTPTimeout) && !errors.Is(err, blockchain.ErrUndefinedExecutionEngineError) { + if blockchain.IsInvalidBlock(err) { interop.WriteBlockToDisk(signed, true /*failed*/) s.setBadBlock(ctx, root) } diff --git a/beacon-chain/sync/validate_beacon_blocks.go b/beacon-chain/sync/validate_beacon_blocks.go index 1a5df208a0..5198efd979 100644 --- a/beacon-chain/sync/validate_beacon_blocks.go +++ b/beacon-chain/sync/validate_beacon_blocks.go @@ -8,7 +8,6 @@ import ( "github.com/libp2p/go-libp2p-core/peer" pubsub "github.com/libp2p/go-libp2p-pubsub" "github.com/pkg/errors" - "github.com/prysmaticlabs/prysm/beacon-chain/blockchain" "github.com/prysmaticlabs/prysm/beacon-chain/core/blocks" "github.com/prysmaticlabs/prysm/beacon-chain/core/feed" blockfeed "github.com/prysmaticlabs/prysm/beacon-chain/core/feed/block" @@ -241,7 +240,7 @@ func (s *Service) validateBeaconBlock(ctx context.Context, blk interfaces.Signed } if err = s.validateBellatrixBeaconBlock(ctx, parentState, blk.Block()); err != nil { - if errors.Is(err, ErrOptimisticParent) || errors.Is(blockchain.ErrUndefinedExecutionEngineError, err) { + if errors.Is(err, ErrOptimisticParent) { return err } // for other kinds of errors, set this block as a bad block.