Ensure Block Processing Failures Return an Error (#2325)

* ensure block failed processing returns an error

* fixed test

* test assertion corrected

* comments

* fix tests

* imports

beacon-chain/blockchain/block_processing.go

@@ -100,6 +100,7 @@ func (c *ChainService) ReceiveBlock(ctx context.Context, block *pb.BeaconBlock)
 			if err := c.beaconDB.DeleteBlock(block); err != nil {
 				return nil, fmt.Errorf("could not delete bad block from db: %v", err)
 			}
+			return beaconState, err
 		default:
 			return beaconState, fmt.Errorf("could not apply block state transition: %v", err)
 		}

beacon-chain/blockchain/block_processing_test.go

@@ -15,6 +15,7 @@ import (
 	"github.com/prysmaticlabs/prysm/beacon-chain/internal"
 	pb "github.com/prysmaticlabs/prysm/proto/beacon/p2p/v1"
 	"github.com/prysmaticlabs/prysm/shared/bytesutil"
+	"github.com/prysmaticlabs/prysm/shared/featureconfig"
 	"github.com/prysmaticlabs/prysm/shared/hashutil"
 	"github.com/prysmaticlabs/prysm/shared/params"
 	"github.com/prysmaticlabs/prysm/shared/testutil"
@@ -220,6 +221,9 @@ func TestReceiveBlock_UsesParentBlockState(t *testing.T) {
 }
 
 func TestReceiveBlock_DeletesBadBlock(t *testing.T) {
+	featureconfig.InitFeatureConfig(&featureconfig.FeatureFlagConfig{
+		EnableCheckBlockStateRoot: false,
+	})
 	db := internal.SetupDB(t)
 	defer internal.TeardownDB(t, db)
 	ctx := context.Background()
@@ -274,8 +278,12 @@ func TestReceiveBlock_DeletesBadBlock(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	if _, err := chainService.ReceiveBlock(context.Background(), block); err == nil {
-		t.Error("Expected block to fail processing, received nil")
+	_, err = chainService.ReceiveBlock(context.Background(), block)
+	switch err.(type) {
+	case *BlockFailedProcessingErr:
+		t.Log("Block failed processing as expected")
+	default:
+		t.Errorf("Unexpected block processing error: %v", err)
 	}
 
 	savedBlock, err := db.Block(blockRoot)
@@ -289,6 +297,9 @@ func TestReceiveBlock_DeletesBadBlock(t *testing.T) {
 	if !db.IsEvilBlockHash(blockRoot) {
 		t.Error("Expected block root to have been blacklisted")
 	}
+	featureconfig.InitFeatureConfig(&featureconfig.FeatureFlagConfig{
+		EnableCheckBlockStateRoot: true,
+	})
 }
 
 func TestReceiveBlock_CheckBlockStateRoot_GoodState(t *testing.T) {

beacon-chain/sync/initial-sync/service.go

@@ -212,6 +212,7 @@ func (s *InitialSync) exitInitialSync(ctx context.Context, block *pb.BeaconBlock
 			if err := s.db.DeleteBlock(block); err != nil {
 				return fmt.Errorf("could not delete bad block from db: %v", err)
 			}
+			return fmt.Errorf("could not apply block state transition: %v", err)
 		default:
 			return fmt.Errorf("could not apply block state transition: %v", err)
 		}