From 2ea09b621e33340c2bba1763dfbf38b4ffdd38b2 Mon Sep 17 00:00:00 2001
From: terence tsao <terence@prysmaticlabs.com>
Date: Wed, 3 Nov 2021 15:29:03 -0700
Subject: [PATCH] Simplify should update justified (#9837)

* Simplify should update justified

* Update tests

Co-authored-by: Raul Jordan <raul@prysmaticlabs.com>
Co-authored-by: prylabs-bulldozer[bot] <58059840+prylabs-bulldozer[bot]@users.noreply.github.com>
---
 .../blockchain/process_block_helpers.go       | 56 +++++++------------
 beacon-chain/blockchain/process_block_test.go |  4 +-
 2 files changed, 24 insertions(+), 36 deletions(-)

diff --git a/beacon-chain/blockchain/process_block_helpers.go b/beacon-chain/blockchain/process_block_helpers.go
index bd89fa598b..7fe3cecca8 100644
--- a/beacon-chain/blockchain/process_block_helpers.go
+++ b/beacon-chain/blockchain/process_block_helpers.go
@@ -135,7 +135,24 @@ func (s *Service) verifyBlkFinalizedSlot(b block.BeaconBlock) error {
 // shouldUpdateCurrentJustified prevents bouncing attack, by only update conflicting justified
 // checkpoints in the fork choice if in the early slots of the epoch.
 // Otherwise, delay incorporation of new justified checkpoint until next epoch boundary.
-// See https://ethresear.ch/t/prevention-of-bouncing-attack-on-ffg/6114 for more detailed analysis and discussion.
+//
+// Spec code:
+// def should_update_justified_checkpoint(store: Store, new_justified_checkpoint: Checkpoint) -> bool:
+//    """
+//    To address the bouncing attack, only update conflicting justified
+//    checkpoints in the fork choice if in the early slots of the epoch.
+//    Otherwise, delay incorporation of new justified checkpoint until next epoch boundary.
+//
+//    See https://ethresear.ch/t/prevention-of-bouncing-attack-on-ffg/6114 for more detailed analysis and discussion.
+//    """
+//    if compute_slots_since_epoch_start(get_current_slot(store)) < SAFE_SLOTS_TO_UPDATE_JUSTIFIED:
+//        return True
+//
+//    justified_slot = compute_start_slot_at_epoch(store.justified_checkpoint.epoch)
+//    if not get_ancestor(store, new_justified_checkpoint.root, justified_slot) == store.justified_checkpoint.root:
+//        return False
+//
+//    return True
 func (s *Service) shouldUpdateCurrentJustified(ctx context.Context, newJustifiedCheckpt *ethpb.Checkpoint) (bool, error) {
 	ctx, span := trace.StartSpan(ctx, "blockChain.shouldUpdateCurrentJustified")
 	defer span.End()
@@ -143,51 +160,20 @@ func (s *Service) shouldUpdateCurrentJustified(ctx context.Context, newJustified
 	if slots.SinceEpochStarts(s.CurrentSlot()) < params.BeaconConfig().SafeSlotsToUpdateJustified {
 		return true, nil
 	}
-	var newJustifiedBlockSigned block.SignedBeaconBlock
-	justifiedRoot := s.ensureRootNotZeros(bytesutil.ToBytes32(newJustifiedCheckpt.Root))
-	var err error
-	if s.hasInitSyncBlock(justifiedRoot) {
-		newJustifiedBlockSigned = s.getInitSyncBlock(justifiedRoot)
-	} else {
-		newJustifiedBlockSigned, err = s.cfg.BeaconDB.Block(ctx, justifiedRoot)
-		if err != nil {
-			return false, err
-		}
-	}
-	if newJustifiedBlockSigned == nil || newJustifiedBlockSigned.IsNil() || newJustifiedBlockSigned.Block().IsNil() {
-		return false, errors.New("nil new justified block")
-	}
 
-	newJustifiedBlock := newJustifiedBlockSigned.Block()
 	jSlot, err := slots.EpochStart(s.justifiedCheckpt.Epoch)
 	if err != nil {
 		return false, err
 	}
-	if newJustifiedBlock.Slot() <= jSlot {
-		return false, nil
-	}
-	var justifiedBlockSigned block.SignedBeaconBlock
-	cachedJustifiedRoot := s.ensureRootNotZeros(bytesutil.ToBytes32(s.justifiedCheckpt.Root))
-	if s.hasInitSyncBlock(cachedJustifiedRoot) {
-		justifiedBlockSigned = s.getInitSyncBlock(cachedJustifiedRoot)
-	} else {
-		justifiedBlockSigned, err = s.cfg.BeaconDB.Block(ctx, cachedJustifiedRoot)
-		if err != nil {
-			return false, err
-		}
-	}
-
-	if justifiedBlockSigned == nil || justifiedBlockSigned.IsNil() || justifiedBlockSigned.Block().IsNil() {
-		return false, errors.New("nil justified block")
-	}
-	justifiedBlock := justifiedBlockSigned.Block()
-	b, err := s.ancestor(ctx, justifiedRoot[:], justifiedBlock.Slot())
+	justifiedRoot := s.ensureRootNotZeros(bytesutil.ToBytes32(newJustifiedCheckpt.Root))
+	b, err := s.ancestor(ctx, justifiedRoot[:], jSlot)
 	if err != nil {
 		return false, err
 	}
 	if !bytes.Equal(b, s.justifiedCheckpt.Root) {
 		return false, nil
 	}
+
 	return true, nil
 }
 
diff --git a/beacon-chain/blockchain/process_block_test.go b/beacon-chain/blockchain/process_block_test.go
index 41d2f45e89..e42418e3f0 100644
--- a/beacon-chain/blockchain/process_block_test.go
+++ b/beacon-chain/blockchain/process_block_test.go
@@ -229,6 +229,8 @@ func TestShouldUpdateJustified_ReturnFalse(t *testing.T) {
 	service, err := NewService(ctx)
 	require.NoError(t, err)
 	service.cfg = cfg
+	service.cfg.ForkChoiceStore = protoarray.New(0, 0, [32]byte{})
+
 	lastJustifiedBlk := util.NewBeaconBlock()
 	lastJustifiedBlk.Block.ParentRoot = bytesutil.PadTo([]byte{'G'}, 32)
 	lastJustifiedRoot, err := lastJustifiedBlk.Block.HashTreeRoot()
@@ -325,7 +327,7 @@ func TestUpdateJustified_CouldUpdateBest(t *testing.T) {
 	ctx := context.Background()
 	beaconDB := testDB.SetupDB(t)
 
-	cfg := &config{BeaconDB: beaconDB, StateGen: stategen.New(beaconDB)}
+	cfg := &config{BeaconDB: beaconDB, StateGen: stategen.New(beaconDB), ForkChoiceStore: protoarray.New(0, 0, [32]byte{})}
 	service, err := NewService(ctx)
 	require.NoError(t, err)
 	service.cfg = cfg