Bounce attack check (#3989)

* New store values * Update process block * Update process attestation * Update tests * Helper * Fixed blockchain package tests * Update beacon-chain/blockchain/forkchoice/process_block.go
prysmaticlabs · Nov 13, 2019 · 6d1a3fa · 6d1a3fa
1 parent d69efa7
commit 6d1a3fa
Show file tree

Hide file tree

Showing 11 changed files with 107 additions and 22 deletions.
diff --git a/beacon-chain/blockchain/chain_info_test.go b/beacon-chain/blockchain/chain_info_test.go
@@ -38,8 +38,11 @@ func TestFinalizedCheckpt_CanRetrieve(t *testing.T) {
 	ctx := context.Background()
 
 	c := setupBeaconChain(t, db)
-
-	if err := c.forkChoiceStore.GenesisStore(ctx, &ethpb.Checkpoint{}, &ethpb.Checkpoint{}); err != nil {
+	r := [32]byte{'g'}
+	if err := db.SaveState(ctx, &pb.BeaconState{}, r); err != nil {
+		t.Fatal(err)
+	}
+	if err := c.forkChoiceStore.GenesisStore(ctx, &ethpb.Checkpoint{Root: r[:]}, &ethpb.Checkpoint{Root: r[:]}); err != nil {
 		t.Fatal(err)
 	}
 

diff --git a/beacon-chain/blockchain/forkchoice/lmd_ghost_yaml_test.go b/beacon-chain/blockchain/forkchoice/lmd_ghost_yaml_test.go
@@ -102,12 +102,10 @@ func TestGetHeadFromYaml(t *testing.T) {
 
 		s := &pb.BeaconState{Validators: validators}
 
-		if err := store.GenesisStore(ctx, &ethpb.Checkpoint{}, &ethpb.Checkpoint{}); err != nil {
+		if err := store.db.SaveState(ctx, s, bytesutil.ToBytes32(blksRoot[0])); err != nil {
 			t.Fatal(err)
 		}
-
-		store.justifiedCheckpt.Root = blksRoot[0]
-		if err := store.db.SaveState(ctx, s, bytesutil.ToBytes32(blksRoot[0])); err != nil {
+		if err := store.GenesisStore(ctx, &ethpb.Checkpoint{Root: blksRoot[0]}, &ethpb.Checkpoint{Root: blksRoot[0]}); err != nil {
 			t.Fatal(err)
 		}
 

diff --git a/beacon-chain/blockchain/forkchoice/process_attestation.go b/beacon-chain/blockchain/forkchoice/process_attestation.go
@@ -137,6 +137,8 @@ func (s *Store) OnAttestation(ctx context.Context, a *ethpb.Attestation) (uint64
 		return 0, err
 	}
 
+	s.updateJustifiedCheckpoint()
+
 	return tgtSlot, nil
 }
 

diff --git a/beacon-chain/blockchain/forkchoice/process_attestation_test.go b/beacon-chain/blockchain/forkchoice/process_attestation_test.go
@@ -93,7 +93,10 @@ func TestStore_OnAttestation(t *testing.T) {
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			if err := store.GenesisStore(ctx, &ethpb.Checkpoint{}, &ethpb.Checkpoint{}); err != nil {
+			if err := store.GenesisStore(
+				ctx,
+				&ethpb.Checkpoint{Root: BlkWithValidStateRoot[:]},
+				&ethpb.Checkpoint{Root: BlkWithValidStateRoot[:]}); err != nil {
 				t.Fatal(err)
 			}
 
@@ -131,7 +134,11 @@ func TestStore_SaveCheckpointState(t *testing.T) {
 		Slashings:           make([]uint64, params.BeaconConfig().EpochsPerSlashingsVector),
 		FinalizedCheckpoint: &ethpb.Checkpoint{},
 	}
-	if err := store.GenesisStore(ctx, &ethpb.Checkpoint{}, &ethpb.Checkpoint{}); err != nil {
+	r := [32]byte{'g'}
+	if err := store.db.SaveState(ctx, s, r); err != nil {
+		t.Fatal(err)
+	}
+	if err := store.GenesisStore(ctx, &ethpb.Checkpoint{Root: r[:]}, &ethpb.Checkpoint{Root: r[:]}); err != nil {
 		t.Fatal(err)
 	}
 
@@ -178,7 +185,7 @@ func TestStore_SaveCheckpointState(t *testing.T) {
 	}
 
 	s.Slot = params.BeaconConfig().SlotsPerEpoch + 1
-	if err := store.GenesisStore(ctx, &ethpb.Checkpoint{}, &ethpb.Checkpoint{}); err != nil {
+	if err := store.GenesisStore(ctx, &ethpb.Checkpoint{Root: r[:]}, &ethpb.Checkpoint{Root: r[:]}); err != nil {
 		t.Fatal(err)
 	}
 	cp3 := &ethpb.Checkpoint{Epoch: 1, Root: []byte{'C'}}

diff --git a/beacon-chain/blockchain/forkchoice/process_block.go b/beacon-chain/blockchain/forkchoice/process_block.go
@@ -5,6 +5,7 @@ import (
 	"context"
 	"encoding/hex"
 	"fmt"
+	"time"
 
 	"github.com/pkg/errors"
 	"github.com/prysmaticlabs/go-ssz"
@@ -89,8 +90,16 @@ func (s *Store) OnBlock(ctx context.Context, b *ethpb.BeaconBlock) error {
 	}
 
 	// Update justified check point.
+	s.updateJustifiedCheckpoint()
 	if postState.CurrentJustifiedCheckpoint.Epoch > s.JustifiedCheckpt().Epoch {
-		s.justifiedCheckpt = postState.CurrentJustifiedCheckpoint
+		s.bestJustifiedCheckpt = s.justifiedCheckpt
+		canUpdate, err := s.shouldUpdateJustified(ctx, postState.CurrentJustifiedCheckpoint)
+		if err != nil {
+			return err
+		}
+		if canUpdate {
+			s.justifiedCheckpt = postState.CurrentJustifiedCheckpoint
+		}
 		if err := s.db.SaveJustifiedCheckpoint(ctx, postState.CurrentJustifiedCheckpoint); err != nil {
 			return errors.Wrap(err, "could not save justified checkpoint")
 		}
@@ -440,3 +449,47 @@ func (s *Store) rmStatesOlderThanLastFinalized(ctx context.Context, startSlot ui
 
 	return nil
 }
+
+// shouldUpdateJustified 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.
+func (s *Store) shouldUpdateJustified(ctx context.Context, newJustifiedCheckpt *ethpb.Checkpoint) (bool, error) {
+	if helpers.SlotsSinceEpochStarts(s.currentSlot()) < params.BeaconConfig().SafeSlotsToUpdateJustified {
+		return true, nil
+	}
+	newJustifiedBlock, err := s.db.Block(ctx, bytesutil.ToBytes32(newJustifiedCheckpt.Root))
+	if err != nil {
+		return false, err
+	}
+	if newJustifiedBlock.Slot <= helpers.StartSlot(s.justifiedCheckpt.Epoch) {
+		return false, nil
+	}
+	justifiedBlock, err := s.db.Block(ctx, bytesutil.ToBytes32(s.justifiedCheckpt.Root))
+	if err != nil {
+		return false, err
+	}
+	b, err := s.ancestor(ctx, newJustifiedCheckpt.Root, justifiedBlock.Slot)
+	if err != nil {
+		return false, err
+	}
+	if !bytes.Equal(b, s.justifiedCheckpt.Root) {
+		return false, nil
+	}
+	return true, nil
+}
+
+// currentSlot returns the current slot based on time.
+func (s *Store) currentSlot() uint64 {
+	return (uint64(time.Now().Unix()) - s.genesisTime) / params.BeaconConfig().SecondsPerSlot
+}
+
+// updates justified check point in store if a better check point is known
+func (s *Store) updateJustifiedCheckpoint() {
+	if helpers.SlotsSinceEpochStarts(s.currentSlot()) == 0 {
+		return
+	}
+	if s.bestJustifiedCheckpt.Epoch > s.justifiedCheckpt.Epoch {
+		s.justifiedCheckpt = s.bestJustifiedCheckpt
+	}
+}
diff --git a/beacon-chain/blockchain/forkchoice/process_block_test.go b/beacon-chain/blockchain/forkchoice/process_block_test.go
@@ -39,16 +39,16 @@ func TestStore_OnBlock(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	randomParentRoot := []byte{'a'}
-	if err := store.db.SaveState(ctx, &pb.BeaconState{}, bytesutil.ToBytes32(randomParentRoot)); err != nil {
+	randomParentRoot := [32]byte{'a'}
+	if err := store.db.SaveState(ctx, &pb.BeaconState{}, randomParentRoot); err != nil {
 		t.Fatal(err)
 	}
 	randomParentRoot2 := roots[1]
 	if err := store.db.SaveState(ctx, &pb.BeaconState{}, bytesutil.ToBytes32(randomParentRoot2)); err != nil {
 		t.Fatal(err)
 	}
-	validGenesisRoot := []byte{'g'}
-	if err := store.db.SaveState(ctx, &pb.BeaconState{}, bytesutil.ToBytes32(validGenesisRoot)); err != nil {
+	validGenesisRoot := [32]byte{'g'}
+	if err := store.db.SaveState(ctx, &pb.BeaconState{}, validGenesisRoot); err != nil {
 		t.Fatal(err)
 	}
 
@@ -67,13 +67,13 @@ func TestStore_OnBlock(t *testing.T) {
 		},
 		{
 			name:          "block is from the feature",
-			blk:           &ethpb.BeaconBlock{ParentRoot: randomParentRoot, Slot: params.BeaconConfig().FarFutureEpoch},
+			blk:           &ethpb.BeaconBlock{ParentRoot: randomParentRoot[:], Slot: params.BeaconConfig().FarFutureEpoch},
 			s:             &pb.BeaconState{},
 			wantErrString: "could not process slot from the future",
 		},
 		{
 			name:          "could not get finalized block",
-			blk:           &ethpb.BeaconBlock{ParentRoot: randomParentRoot},
+			blk:           &ethpb.BeaconBlock{ParentRoot: randomParentRoot[:]},
 			s:             &pb.BeaconState{},
 			wantErrString: "block from slot 0 is not a descendent of the current finalized block",
 		},
@@ -87,7 +87,7 @@ func TestStore_OnBlock(t *testing.T) {
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			if err := store.GenesisStore(ctx, &ethpb.Checkpoint{}, &ethpb.Checkpoint{}); err != nil {
+			if err := store.GenesisStore(ctx, &ethpb.Checkpoint{Root: validGenesisRoot[:]}, &ethpb.Checkpoint{Root: validGenesisRoot[:]}); err != nil {
 				t.Fatal(err)
 			}
 			store.finalizedCheckpt.Root = roots[0]

diff --git a/beacon-chain/blockchain/forkchoice/service.go b/beacon-chain/blockchain/forkchoice/service.go
@@ -43,6 +43,8 @@ type Store struct {
 	attsQueueLock        sync.Mutex
 	seenAtts             map[[32]byte]bool
 	seenAttsLock         sync.Mutex
+	genesisTime          uint64
+	bestJustifiedCheckpt *ethpb.Checkpoint
 }
 
 // NewForkChoiceService instantiates a new service instance that will
@@ -82,6 +84,7 @@ func (s *Store) GenesisStore(
 	finalizedCheckpoint *ethpb.Checkpoint) error {
 
 	s.justifiedCheckpt = proto.Clone(justifiedCheckpoint).(*ethpb.Checkpoint)
+	s.bestJustifiedCheckpt = proto.Clone(justifiedCheckpoint).(*ethpb.Checkpoint)
 	s.finalizedCheckpt = proto.Clone(finalizedCheckpoint).(*ethpb.Checkpoint)
 	s.prevFinalizedCheckpt = proto.Clone(finalizedCheckpoint).(*ethpb.Checkpoint)
 
@@ -97,6 +100,8 @@ func (s *Store) GenesisStore(
 		return errors.Wrap(err, "could not save genesis state in check point cache")
 	}
 
+	s.genesisTime = justifiedState.GenesisTime
+
 	return nil
 }
 

diff --git a/beacon-chain/blockchain/forkchoice/service_test.go b/beacon-chain/blockchain/forkchoice/service_test.go
@@ -270,6 +270,9 @@ func TestStore_GetHead(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
+	if err := store.db.SaveState(ctx, s, blkRoot); err != nil {
+		t.Fatal(err)
+	}
 
 	checkPoint := &ethpb.Checkpoint{Root: blkRoot[:]}
 

diff --git a/beacon-chain/blockchain/receive_block_test.go b/beacon-chain/blockchain/receive_block_test.go
@@ -44,6 +44,9 @@ func TestReceiveBlock_ProcessCorrectly(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
+	if err := db.SaveState(ctx, beaconState, genesisBlkRoot); err != nil {
+		t.Fatal(err)
+	}
 	cp := &ethpb.Checkpoint{Root: genesisBlkRoot[:]}
 	if err := chainService.forkChoiceStore.GenesisStore(ctx, cp, cp); err != nil {
 		t.Fatal(err)
@@ -203,7 +206,14 @@ func TestReceiveBlockNoPubsubForkchoice_ProcessCorrectly(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
-	if err := chainService.forkChoiceStore.GenesisStore(ctx, &ethpb.Checkpoint{}, &ethpb.Checkpoint{}); err != nil {
+	parentRoot, err := ssz.SigningRoot(genesis)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if err := db.SaveState(ctx, beaconState, parentRoot); err != nil {
+		t.Fatal(err)
+	}
+	if err := chainService.forkChoiceStore.GenesisStore(ctx, &ethpb.Checkpoint{Root: parentRoot[:]}, &ethpb.Checkpoint{Root: parentRoot[:]}); err != nil {
 		t.Fatal(err)
 	}
 
@@ -216,10 +226,6 @@ func TestReceiveBlockNoPubsubForkchoice_ProcessCorrectly(t *testing.T) {
 	if err := chainService.beaconDB.SaveBlock(ctx, genesis); err != nil {
 		t.Fatalf("Could not save block to db: %v", err)
 	}
-	parentRoot, err := ssz.SigningRoot(genesis)
-	if err != nil {
-		t.Fatal(err)
-	}
 
 	if err := db.SaveState(ctx, beaconState, parentRoot); err != nil {
 		t.Fatal(err)

diff --git a/beacon-chain/core/helpers/slot_epoch.go b/beacon-chain/core/helpers/slot_epoch.go
@@ -83,6 +83,11 @@ func IsEpochEnd(slot uint64) bool {
 	return IsEpochStart(slot + 1)
 }
 
+// SlotsSinceEpochStarts returns number of slots since the start of the epoch.
+func SlotsSinceEpochStarts(slot uint64) uint64 {
+	return slot - StartSlot(SlotToEpoch(slot))
+}
+
 // Allow for slots "from the future" within a certain tolerance.
 const timeShiftTolerance = 10 // ms
 

diff --git a/shared/params/config.go b/shared/params/config.go
@@ -49,6 +49,7 @@ type BeaconChainConfig struct {
 	PersistentCommitteePeriod        uint64 `yaml:"PERSISTENT_COMMITTEE_PERIOD"`         // PersistentCommitteePeriod is the minimum amount of epochs a validator must participate before exitting.
 	MinEpochsToInactivityPenalty     uint64 `yaml:"MIN_EPOCHS_TO_INACTIVITY_PENALTY"`    // MinEpochsToInactivityPenalty defines the minimum amount of epochs since finality to begin penalizing inactivity.
 	Eth1FollowDistance               uint64 // Eth1FollowDistance is the number of eth1.0 blocks to wait before considering a new deposit for voting. This only applies after the chain as been started.
+	SafeSlotsToUpdateJustified       uint64 // SafeSlotsToUpdateJustified is the minimal slots needed to update justified check point.
 
 	// State list lengths
 	EpochsPerHistoricalVector uint64 `yaml:"EPOCHS_PER_HISTORICAL_VECTOR"` // EpochsPerHistoricalVector defines max length in epoch to store old historical stats in beacon state.
@@ -143,6 +144,7 @@ var defaultBeaconConfig = &BeaconChainConfig{
 	PersistentCommitteePeriod:        2048,
 	MinEpochsToInactivityPenalty:     4,
 	Eth1FollowDistance:               1024,
+	SafeSlotsToUpdateJustified:       8,
 
 	// State list length constants.
 	EpochsPerHistoricalVector: 65536,
@@ -265,6 +267,7 @@ func MinimalSpecConfig() *BeaconChainConfig {
 	minimalConfig.MinValidatorWithdrawabilityDelay = 256
 	minimalConfig.PersistentCommitteePeriod = 2048
 	minimalConfig.MinEpochsToInactivityPenalty = 4
+	minimalConfig.SafeSlotsToUpdateJustified = 2
 
 	// State vector lengths
 	minimalConfig.EpochsPerHistoricalVector = 64