Process attestation: reduce checkpoint copies (#8409)

* Clean up process attestation

* Add matching getters

* Fix tests

* Update tests

* Fix test

* Remove read locks

* Typo

Co-authored-by: Raul Jordan <raul@prysmaticlabs.com>
Co-authored-by: prylabs-bulldozer[bot] <58059840+prylabs-bulldozer[bot]@users.noreply.github.com>

beacon-chain/core/blocks/BUILD.bazel

@@ -40,7 +40,6 @@ go_library(
         "//shared/trieutil:go_default_library",
         "@com_github_gogo_protobuf//proto:go_default_library",
         "@com_github_pkg_errors//:go_default_library",
-        "@com_github_prysmaticlabs_eth2_types//:go_default_library",
         "@com_github_prysmaticlabs_ethereumapis//eth/v1alpha1:go_default_library",
         "@com_github_sirupsen_logrus//:go_default_library",
         "@io_opencensus_go//trace:go_default_library",

beacon-chain/core/blocks/attestation.go

@@ -1,12 +1,10 @@
 package blocks
 
 import (
-	"bytes"
 	"context"
 	"fmt"
 
 	"github.com/pkg/errors"
-	"github.com/prysmaticlabs/eth2-types"
 	ethpb "github.com/prysmaticlabs/ethereumapis/eth/v1alpha1"
 	"github.com/prysmaticlabs/prysm/beacon-chain/core/helpers"
 	stateTrie "github.com/prysmaticlabs/prysm/beacon-chain/state"
@@ -113,13 +111,8 @@ func ProcessAttestationNoVerifySignature(
 	if err := helpers.ValidateNilAttestation(att); err != nil {
 		return nil, err
 	}
-	currEpoch := helpers.SlotToEpoch(beaconState.Slot())
-	var prevEpoch types.Epoch
-	if currEpoch == 0 {
-		prevEpoch = 0
-	} else {
-		prevEpoch = currEpoch - 1
-	}
+	currEpoch := helpers.CurrentEpoch(beaconState)
+	prevEpoch := helpers.PrevEpoch(beaconState)
 	data := att.Data
 	if data.Target.Epoch != prevEpoch && data.Target.Epoch != currEpoch {
 		return nil, fmt.Errorf(
@@ -176,33 +169,21 @@ func ProcessAttestationNoVerifySignature(
 		ProposerIndex:   proposerIndex,
 	}
 
-	var ffgSourceEpoch types.Epoch
-	var ffgSourceRoot []byte
-	var ffgTargetEpoch types.Epoch
 	if data.Target.Epoch == currEpoch {
-		ffgSourceEpoch = beaconState.CurrentJustifiedCheckpoint().Epoch
-		ffgSourceRoot = beaconState.CurrentJustifiedCheckpoint().Root
-		ffgTargetEpoch = currEpoch
+		if !beaconState.MatchCurrentJustifiedCheckpoint(data.Source) {
+			return nil, errors.New("source check point not equal to current justified checkpoint")
+		}
 		if err := beaconState.AppendCurrentEpochAttestations(pendingAtt); err != nil {
 			return nil, err
 		}
 	} else {
-		ffgSourceEpoch = beaconState.PreviousJustifiedCheckpoint().Epoch
-		ffgSourceRoot = beaconState.PreviousJustifiedCheckpoint().Root
-		ffgTargetEpoch = prevEpoch
+		if !beaconState.MatchPreviousJustifiedCheckpoint(data.Source) {
+			return nil, errors.New("source check point not equal to previous justified checkpoint")
+		}
 		if err := beaconState.AppendPreviousEpochAttestations(pendingAtt); err != nil {
 			return nil, err
 		}
 	}
-	if data.Source.Epoch != ffgSourceEpoch {
-		return nil, fmt.Errorf("expected source epoch %d, received %d", ffgSourceEpoch, data.Source.Epoch)
-	}
-	if !bytes.Equal(data.Source.Root, ffgSourceRoot) {
-		return nil, fmt.Errorf("expected source root %#x, received %#x", ffgSourceRoot, data.Source.Root)
-	}
-	if data.Target.Epoch != ffgTargetEpoch {
-		return nil, fmt.Errorf("expected target epoch %d, received %d", ffgTargetEpoch, data.Target.Epoch)
-	}
 
 	// Verify attesting indices are correct.
 	committee, err := helpers.BeaconCommitteeFromState(beaconState, att.Data.Slot, att.Data.CommitteeIndex)

beacon-chain/core/blocks/attestation_test.go

@@ -104,22 +104,11 @@ func TestProcessAttestations_CurrentEpochFFGDataMismatches(t *testing.T) {
 	require.NoError(t, beaconState.SetCurrentJustifiedCheckpoint(cfc))
 	require.NoError(t, beaconState.SetCurrentEpochAttestations([]*pb.PendingAttestation{}))
 
-	want := fmt.Sprintf(
-		"expected source epoch %d, received %d",
-		helpers.CurrentEpoch(beaconState),
-		attestations[0].Data.Source.Epoch,
-	)
+	want := "source check point not equal to current justified checkpoint"
 	_, err := blocks.ProcessAttestations(context.Background(), beaconState, b)
 	assert.ErrorContains(t, want, err)
-
 	b.Block.Body.Attestations[0].Data.Source.Epoch = helpers.CurrentEpoch(beaconState)
 	b.Block.Body.Attestations[0].Data.Source.Root = []byte{}
-
-	want = fmt.Sprintf(
-		"expected source root %#x, received %#x",
-		beaconState.CurrentJustifiedCheckpoint().Root,
-		attestations[0].Data.Source.Root,
-	)
 	_, err = blocks.ProcessAttestations(context.Background(), beaconState, b)
 	assert.ErrorContains(t, want, err)
 }
@@ -146,30 +135,19 @@ func TestProcessAttestations_PrevEpochFFGDataMismatches(t *testing.T) {
 		},
 	}
 
-	err := beaconState.SetSlot(beaconState.Slot() + params.BeaconConfig().SlotsPerEpoch + params.BeaconConfig().MinAttestationInclusionDelay)
+	err := beaconState.SetSlot(beaconState.Slot() + 2*params.BeaconConfig().SlotsPerEpoch)
 	require.NoError(t, err)
 	pfc := beaconState.PreviousJustifiedCheckpoint()
 	pfc.Root = []byte("hello-world")
 	require.NoError(t, beaconState.SetPreviousJustifiedCheckpoint(pfc))
 	require.NoError(t, beaconState.SetPreviousEpochAttestations([]*pb.PendingAttestation{}))
 
-	want := fmt.Sprintf(
-		"expected source epoch %d, received %d",
-		helpers.PrevEpoch(beaconState),
-		attestations[0].Data.Source.Epoch,
-	)
+	want := "source check point not equal to previous justified checkpoint"
 	_, err = blocks.ProcessAttestations(context.Background(), beaconState, b)
 	assert.ErrorContains(t, want, err)
-
 	b.Block.Body.Attestations[0].Data.Source.Epoch = helpers.PrevEpoch(beaconState)
-	b.Block.Body.Attestations[0].Data.Target.Epoch = helpers.CurrentEpoch(beaconState)
+	b.Block.Body.Attestations[0].Data.Target.Epoch = helpers.PrevEpoch(beaconState)
 	b.Block.Body.Attestations[0].Data.Source.Root = []byte{}
-
-	want = fmt.Sprintf(
-		"expected source root %#x, received %#x",
-		beaconState.CurrentJustifiedCheckpoint().Root,
-		attestations[0].Data.Source.Root,
-	)
 	_, err = blocks.ProcessAttestations(context.Background(), beaconState, b)
 	assert.ErrorContains(t, want, err)
 }

beacon-chain/state/getters.go

@@ -1,6 +1,7 @@
 package state
 
 import (
+	"bytes"
 	"errors"
 	"fmt"
 	"time"
@@ -1017,6 +1018,38 @@ func (b *BeaconState) currentJustifiedCheckpoint() *ethpb.Checkpoint {
 	return b.safeCopyCheckpoint(b.state.CurrentJustifiedCheckpoint)
 }
 
+// MatchCurrentJustifiedCheckpoint returns true if input justified checkpoint matches
+// the current justified checkpoint in state.
+func (b *BeaconState) MatchCurrentJustifiedCheckpoint(c *ethpb.Checkpoint) bool {
+	if !b.HasInnerState() {
+		return false
+	}
+	if b.state.CurrentJustifiedCheckpoint == nil {
+		return false
+	}
+
+	if c.Epoch != b.state.CurrentJustifiedCheckpoint.Epoch {
+		return false
+	}
+	return bytes.Equal(c.Root, b.state.CurrentJustifiedCheckpoint.Root)
+}
+
+// MatchPreviousJustifiedCheckpoint returns true if the input justified checkpoint matches
+// the previous justified checkpoint in state.
+func (b *BeaconState) MatchPreviousJustifiedCheckpoint(c *ethpb.Checkpoint) bool {
+	if !b.HasInnerState() {
+		return false
+	}
+	if b.state.PreviousJustifiedCheckpoint == nil {
+		return false
+	}
+
+	if c.Epoch != b.state.PreviousJustifiedCheckpoint.Epoch {
+		return false
+	}
+	return bytes.Equal(c.Root, b.state.PreviousJustifiedCheckpoint.Root)
+}
+
 // FinalizedCheckpoint denoting an epoch and block root.
 func (b *BeaconState) FinalizedCheckpoint() *ethpb.Checkpoint {
 	if !b.HasInnerState() {

beacon-chain/state/getters_test.go

@@ -89,3 +89,29 @@ func TestReadOnlyValidator_ActivationEligibilityEpochNoPanic(t *testing.T) {
 	v := &ReadOnlyValidator{}
 	assert.Equal(t, types.Epoch(0), v.ActivationEligibilityEpoch(), "Expected 0 and not panic")
 }
+
+func TestBeaconState_MatchCurrentJustifiedCheckpt(t *testing.T) {
+	c1 := &eth.Checkpoint{Epoch: 1}
+	c2 := &eth.Checkpoint{Epoch: 2}
+	state, err := InitializeFromProto(&pb.BeaconState{CurrentJustifiedCheckpoint: c1})
+	require.NoError(t, err)
+	require.Equal(t, true, state.MatchCurrentJustifiedCheckpoint(c1))
+	require.Equal(t, false, state.MatchCurrentJustifiedCheckpoint(c2))
+	require.Equal(t, false, state.MatchPreviousJustifiedCheckpoint(c1))
+	require.Equal(t, false, state.MatchPreviousJustifiedCheckpoint(c2))
+	state.state = nil
+	require.Equal(t, false, state.MatchCurrentJustifiedCheckpoint(c1))
+}
+
+func TestBeaconState_MatchPreviousJustifiedCheckpt(t *testing.T) {
+	c1 := &eth.Checkpoint{Epoch: 1}
+	c2 := &eth.Checkpoint{Epoch: 2}
+	state, err := InitializeFromProto(&pb.BeaconState{PreviousJustifiedCheckpoint: c1})
+	require.NoError(t, err)
+	require.Equal(t, false, state.MatchCurrentJustifiedCheckpoint(c1))
+	require.Equal(t, false, state.MatchCurrentJustifiedCheckpoint(c2))
+	require.Equal(t, true, state.MatchPreviousJustifiedCheckpoint(c1))
+	require.Equal(t, false, state.MatchPreviousJustifiedCheckpoint(c2))
+	state.state = nil
+	require.Equal(t, false, state.MatchPreviousJustifiedCheckpoint(c1))
+}