From 1df15f72c4b28e6a415d16686c21fd618e16a1ad Mon Sep 17 00:00:00 2001
From: Raul Jordan <raul@prysmaticlabs.com>
Date: Sun, 21 Apr 2019 17:38:14 -0500
Subject: [PATCH] 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      |  1 +
 beacon-chain/blockchain/block_processing_test.go | 15 +++++++++++++--
 beacon-chain/sync/initial-sync/service.go        |  1 +
 3 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/beacon-chain/blockchain/block_processing.go b/beacon-chain/blockchain/block_processing.go
index e2251f3e93f7..29f4d5a5887c 100644
--- a/beacon-chain/blockchain/block_processing.go
+++ b/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)
 		}
diff --git a/beacon-chain/blockchain/block_processing_test.go b/beacon-chain/blockchain/block_processing_test.go
index 0b3db19c9221..0d34221e135a 100644
--- a/beacon-chain/blockchain/block_processing_test.go
+++ b/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) {
diff --git a/beacon-chain/sync/initial-sync/service.go b/beacon-chain/sync/initial-sync/service.go
index f8b4929da534..12678db26b84 100644
--- a/beacon-chain/sync/initial-sync/service.go
+++ b/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)
 		}