From 9830ce43d6a786959f147c1e56cf83bfd491988e Mon Sep 17 00:00:00 2001
From: terencechain <terence@prysmaticlabs.com>
Date: Mon, 25 Jul 2022 06:45:03 -0700
Subject: [PATCH] Set invalid roots for bad block (#10982)

* Set invalid roots for bad block

* Update for fcu

* Update for fcu

* Set bad blocks in subscriber

* Update process_block_test.go

* Rename
---
 beacon-chain/blockchain/error.go              | 19 +++++++++++++++++++
 beacon-chain/blockchain/error_test.go         | 11 +++++++++++
 beacon-chain/blockchain/execution_engine.go   |  7 +++++--
 beacon-chain/blockchain/process_block_test.go |  2 +-
 beacon-chain/sync/subscriber_beacon_blocks.go |  4 ++++
 5 files changed, 40 insertions(+), 3 deletions(-)

diff --git a/beacon-chain/blockchain/error.go b/beacon-chain/blockchain/error.go
index bd260d02fa..874eb0ba90 100644
--- a/beacon-chain/blockchain/error.go
+++ b/beacon-chain/blockchain/error.go
@@ -40,12 +40,14 @@ var (
 // 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 {
+	invalidAncestorRoots [][32]byte
 	error
 	root [32]byte
 }
 
 type invalidBlockError interface {
 	Error() string
+	InvalidAncestorRoots() [][32]byte
 	BlockRoot() [32]byte
 }
 
@@ -54,6 +56,11 @@ func (e invalidBlock) BlockRoot() [32]byte {
 	return e.root
 }
 
+// InvalidAncestorRoots returns an optional list of invalid roots of the invalid block which leads up last valid root.
+func (e invalidBlock) InvalidAncestorRoots() [][32]byte {
+	return e.invalidAncestorRoots
+}
+
 // IsInvalidBlock returns true if the error has `invalidBlock`.
 func IsInvalidBlock(e error) bool {
 	if e == nil {
@@ -78,3 +85,15 @@ func InvalidBlockRoot(e error) [32]byte {
 	}
 	return d.BlockRoot()
 }
+
+// InvalidAncestorRoots returns a list of invalid roots up to last valid root.
+func InvalidAncestorRoots(e error) [][32]byte {
+	if e == nil {
+		return [][32]byte{}
+	}
+	d, ok := e.(invalidBlockError)
+	if !ok {
+		return [][32]byte{}
+	}
+	return d.InvalidAncestorRoots()
+}
diff --git a/beacon-chain/blockchain/error_test.go b/beacon-chain/blockchain/error_test.go
index 1cced67148..0c11940c5f 100644
--- a/beacon-chain/blockchain/error_test.go
+++ b/beacon-chain/blockchain/error_test.go
@@ -14,6 +14,7 @@ func TestIsInvalidBlock(t *testing.T) {
 
 	newErr := errors.Wrap(err, "wrap me")
 	require.Equal(t, true, IsInvalidBlock(newErr))
+	require.DeepEqual(t, [][32]byte(nil), InvalidAncestorRoots(err))
 }
 
 func TestInvalidBlockRoot(t *testing.T) {
@@ -22,4 +23,14 @@ func TestInvalidBlockRoot(t *testing.T) {
 
 	err := invalidBlock{error: ErrInvalidPayload, root: [32]byte{'a'}}
 	require.Equal(t, [32]byte{'a'}, InvalidBlockRoot(err))
+	require.DeepEqual(t, [][32]byte(nil), InvalidAncestorRoots(err))
+}
+
+func TestInvalidRoots(t *testing.T) {
+	roots := [][32]byte{{'d'}, {'b'}, {'c'}}
+	err := invalidBlock{error: ErrInvalidPayload, root: [32]byte{'a'}, invalidAncestorRoots: roots}
+
+	require.Equal(t, true, IsInvalidBlock(err))
+	require.Equal(t, [32]byte{'a'}, InvalidBlockRoot(err))
+	require.DeepEqual(t, roots, InvalidAncestorRoots(err))
 }
diff --git a/beacon-chain/blockchain/execution_engine.go b/beacon-chain/blockchain/execution_engine.go
index ddb9093a7e..b970c0a150 100644
--- a/beacon-chain/blockchain/execution_engine.go
+++ b/beacon-chain/blockchain/execution_engine.go
@@ -133,7 +133,7 @@ func (s *Service) notifyForkchoiceUpdate(ctx context.Context, arg *notifyForkcho
 				"invalidCount": len(invalidRoots),
 				"newHeadRoot":  fmt.Sprintf("%#x", bytesutil.Trunc(r[:])),
 			}).Warn("Pruned invalid blocks")
-			return pid, invalidBlock{error: ErrInvalidPayload, root: arg.headRoot}
+			return pid, invalidBlock{error: ErrInvalidPayload, root: arg.headRoot, invalidAncestorRoots: invalidRoots}
 
 		default:
 			log.WithError(err).Error(ErrUndefinedExecutionEngineError)
@@ -232,7 +232,10 @@ 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{
+			invalidAncestorRoots: invalidRoots,
+			error:                ErrInvalidPayload,
+		}
 	case powchain.ErrInvalidBlockHashPayloadStatus:
 		newPayloadInvalidNodeCount.Inc()
 		return false, ErrInvalidBlockHashPayloadStatus
diff --git a/beacon-chain/blockchain/process_block_test.go b/beacon-chain/blockchain/process_block_test.go
index af22691758..3963d315a0 100644
--- a/beacon-chain/blockchain/process_block_test.go
+++ b/beacon-chain/blockchain/process_block_test.go
@@ -779,7 +779,7 @@ func TestFillForkChoiceMissingBlocks_FinalizedSibling_DoublyLinkedTree(t *testin
 
 	err = service.fillInForkChoiceMissingBlocks(
 		context.Background(), wsb.Block(), beaconState.FinalizedCheckpoint(), beaconState.CurrentJustifiedCheckpoint())
-	require.ErrorIs(t, errNotDescendantOfFinalized, err)
+	require.Equal(t, errNotDescendantOfFinalized.Error(), err.Error())
 }
 
 // blockTree1 constructs the following tree:
diff --git a/beacon-chain/sync/subscriber_beacon_blocks.go b/beacon-chain/sync/subscriber_beacon_blocks.go
index b518a35ea8..fd64a1e913 100644
--- a/beacon-chain/sync/subscriber_beacon_blocks.go
+++ b/beacon-chain/sync/subscriber_beacon_blocks.go
@@ -39,6 +39,10 @@ func (s *Service) beaconBlockSubscriber(ctx context.Context, msg proto.Message)
 				s.setBadBlock(ctx, root)
 			}
 		}
+		// Set the returned invalid ancestors as bad.
+		for _, root := range blockchain.InvalidAncestorRoots(err) {
+			s.setBadBlock(ctx, root)
+		}
 		return err
 	}
 	return err