Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Decoy flip flop check #3987

Merged
merged 3 commits into from
Nov 13, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions beacon-chain/blockchain/forkchoice/process_attestation.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,11 @@ func (s *Store) OnAttestation(ctx context.Context, a *ethpb.Attestation) (uint64
return 0, err
}

// Verify attestation is from current epoch or previous epoch.
if err := s.verifyAttTargetEpoch(ctx, baseState.GenesisTime, uint64(time.Now().Unix()), tgt); err != nil {
return 0, err
}

// Verify Attestations cannot be from future epochs.
if err := helpers.VerifySlotTime(baseState.GenesisTime, tgtSlot); err != nil {
return 0, errors.Wrap(err, "could not verify attestation target slot")
Expand Down Expand Up @@ -147,6 +152,21 @@ func (s *Store) verifyAttPreState(ctx context.Context, c *ethpb.Checkpoint) (*pb
return baseState, nil
}

// verifyAttTargetEpoch validates attestation is from the current or previous epoch.
func (s *Store) verifyAttTargetEpoch(ctx context.Context, genesisTime uint64, nowTime uint64, c *ethpb.Checkpoint) error {
currentSlot := (nowTime - genesisTime) / params.BeaconConfig().SecondsPerSlot
currentEpoch := helpers.SlotToEpoch(currentSlot)
var prevEpoch uint64
// Prevents previous epoch under flow
if currentEpoch > 1 {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shouldnt this be currentEpoch > 0 ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

prevEpoch is 0 by default, the only time when prevEpoch can be 1 or more is when currentEpoch is 2 and more

I mostly followed the spec PR there, currentEpoch > 0 works too

prevEpoch = currentEpoch - 1
}
if c.Epoch != prevEpoch && c.Epoch != currentEpoch {
return fmt.Errorf("target epoch %d does not match current epoch %d or prev epoch %d", c.Epoch, currentEpoch, prevEpoch)
}
return nil
}

// saveCheckpointState saves and returns the processed state with the associated check point.
func (s *Store) saveCheckpointState(ctx context.Context, baseState *pb.BeaconState, c *ethpb.Checkpoint) (*pb.BeaconState, error) {
s.checkpointStateLock.Lock()
Expand Down
50 changes: 48 additions & 2 deletions beacon-chain/blockchain/forkchoice/process_attestation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -82,12 +82,12 @@ func TestStore_OnAttestation(t *testing.T) {
wantErrString: "pre state of target block 0 does not exist",
},
{
name: "process attestation from future epoch",
name: "process attestation doesn't match current epoch",
a: &ethpb.Attestation{Data: &ethpb.AttestationData{Target: &ethpb.Checkpoint{Epoch: params.BeaconConfig().FarFutureEpoch,
Root: BlkWithStateBadAttRoot[:]}}},
s: &pb.BeaconState{},
wantErr: true,
wantErrString: "could not process slot from the future",
wantErrString: "does not match current epoch",
},
}

Expand Down Expand Up @@ -258,3 +258,49 @@ func TestStore_ReturnAggregatedAttestation(t *testing.T) {
t.Error("did not retrieve saved attestation")
}
}

func TestAttEpoch_MatchPrevEpoch(t *testing.T) {
ctx := context.Background()
db := testDB.SetupDB(t)
defer testDB.TeardownDB(t, db)

store := NewForkChoiceService(ctx, db)
if err := store.verifyAttTargetEpoch(
ctx,
0,
params.BeaconConfig().SlotsPerEpoch*params.BeaconConfig().SecondsPerSlot,
&ethpb.Checkpoint{}); err != nil {
t.Error(err)
}
}

func TestAttEpoch_MatchCurrentEpoch(t *testing.T) {
ctx := context.Background()
db := testDB.SetupDB(t)
defer testDB.TeardownDB(t, db)

store := NewForkChoiceService(ctx, db)
if err := store.verifyAttTargetEpoch(
ctx,
0,
params.BeaconConfig().SlotsPerEpoch*params.BeaconConfig().SecondsPerSlot,
&ethpb.Checkpoint{Epoch: 1}); err != nil {
t.Error(err)
}
}

func TestAttEpoch_NotMatch(t *testing.T) {
ctx := context.Background()
db := testDB.SetupDB(t)
defer testDB.TeardownDB(t, db)

store := NewForkChoiceService(ctx, db)
if err := store.verifyAttTargetEpoch(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

long line to follow the err. maybe its better to split it ?

ctx,
0,
2*params.BeaconConfig().SlotsPerEpoch*params.BeaconConfig().SecondsPerSlot,
&ethpb.Checkpoint{}); !strings.Contains(err.Error(),
"target epoch 0 does not match current epoch 2 or prev epoch 1") {
t.Error("Did not receive wanted error")
}
}