Revert "Revert "Correct duties calculation for old slots (prysmaticla…

…bs#11723)"" This reverts commit cab0f5e.
PatriceVignola · Dec 21, 2022 · bfc3617 · bfc3617
1 parent cab0f5e
commit bfc3617
Show file tree

Hide file tree

Showing 2 changed files with 60 additions and 156 deletions.
diff --git a/beacon-chain/rpc/eth/validator/validator.go b/beacon-chain/rpc/eth/validator/validator.go
@@ -15,7 +15,6 @@ import (
 	"github.com/prysmaticlabs/prysm/v3/beacon-chain/builder"
 	"github.com/prysmaticlabs/prysm/v3/beacon-chain/cache"
 	"github.com/prysmaticlabs/prysm/v3/beacon-chain/core/helpers"
-	"github.com/prysmaticlabs/prysm/v3/beacon-chain/core/transition"
 	"github.com/prysmaticlabs/prysm/v3/beacon-chain/db/kv"
 	rpchelpers "github.com/prysmaticlabs/prysm/v3/beacon-chain/rpc/eth/helpers"
 	"github.com/prysmaticlabs/prysm/v3/beacon-chain/state"
@@ -61,13 +60,13 @@ func (vs *Server) GetAttesterDuties(ctx context.Context, req *ethpbv1.AttesterDu
 		return nil, status.Errorf(codes.Internal, "Could not check optimistic status: %v", err)
 	}
 
-	s, err := vs.HeadFetcher.HeadState(ctx)
+	startSlot, err := slots.EpochStart(req.Epoch)
 	if err != nil {
-		return nil, status.Errorf(codes.Internal, "Could not get head state: %v", err)
+		return nil, status.Errorf(codes.Internal, "Could not get start slot from epoch: %v", err)
 	}
-	s, err = advanceState(ctx, s, req.Epoch, currentEpoch)
+	s, err := vs.StateFetcher.StateBySlot(ctx, startSlot)
 	if err != nil {
-		return nil, status.Errorf(codes.Internal, "Could not advance state to requested epoch start slot: %v", err)
+		return nil, status.Errorf(codes.Internal, "Could not get state: %v", err)
 	}
 
 	committeeAssignments, _, err := helpers.CommitteeAssignments(ctx, s, req.Epoch)
@@ -144,13 +143,13 @@ func (vs *Server) GetProposerDuties(ctx context.Context, req *ethpbv1.ProposerDu
 		return nil, status.Errorf(codes.Internal, "Could not check optimistic status: %v", err)
 	}
 
-	s, err := vs.HeadFetcher.HeadState(ctx)
+	startSlot, err := slots.EpochStart(req.Epoch)
 	if err != nil {
-		return nil, status.Errorf(codes.Internal, "Could not get head state: %v", err)
+		return nil, status.Errorf(codes.Internal, "Could not get start slot from epoch: %v", err)
 	}
-	s, err = advanceState(ctx, s, req.Epoch, currentEpoch)
+	s, err := vs.StateFetcher.StateBySlot(ctx, startSlot)
 	if err != nil {
-		return nil, status.Errorf(codes.Internal, "Could not advance state to requested epoch start slot: %v", err)
+		return nil, status.Errorf(codes.Internal, "Could not get state: %v", err)
 	}
 
 	_, proposals, err := helpers.CommitteeAssignments(ctx, s, req.Epoch)
@@ -179,7 +178,7 @@ func (vs *Server) GetProposerDuties(ctx context.Context, req *ethpbv1.ProposerDu
 		return duties[i].Slot < duties[j].Slot
 	})
 
-	root, err := vs.proposalDependentRoot(ctx, s, req.Epoch)
+	root, err := proposalDependentRoot(s, req.Epoch)
 	if err != nil {
 		return nil, status.Errorf(codes.Internal, "Could not get dependent root: %v", err)
 	}
@@ -1059,7 +1058,7 @@ func attestationDependentRoot(s state.BeaconState, epoch types.Epoch) ([]byte, e
 
 // proposalDependentRoot is get_block_root_at_slot(state, compute_start_slot_at_epoch(epoch) - 1)
 // or the genesis block root in the case of underflow.
-func (vs *Server) proposalDependentRoot(ctx context.Context, s state.BeaconState, epoch types.Epoch) ([]byte, error) {
+func proposalDependentRoot(s state.BeaconState, epoch types.Epoch) ([]byte, error) {
 	var dependentRootSlot types.Slot
 	if epoch == 0 {
 		dependentRootSlot = 0
@@ -1070,47 +1069,11 @@ func (vs *Server) proposalDependentRoot(ctx context.Context, s state.BeaconState
 		}
 		dependentRootSlot = epochStartSlot.Sub(1)
 	}
-	var root []byte
-	var err error
-	// Per spec, if the dependent root epoch is greater than current epoch, use the head root.
-	if dependentRootSlot >= s.Slot() {
-		root, err = vs.HeadFetcher.HeadRoot(ctx)
-		if err != nil {
-			return nil, err
-		}
-	} else {
-		root, err = helpers.BlockRootAtSlot(s, dependentRootSlot)
-		if err != nil {
-			return nil, errors.Wrap(err, "could not get block root")
-		}
-	}
-
-	return root, nil
-}
-
-// advanceState advances state with empty transitions up to the requested epoch start slot.
-// In case 1 epoch ahead was requested, we take the start slot of the current epoch.
-// Taking the start slot of the next epoch would result in an error inside transition.ProcessSlots.
-func advanceState(ctx context.Context, s state.BeaconState, requestedEpoch, currentEpoch types.Epoch) (state.BeaconState, error) {
-	var epochStartSlot types.Slot
-	var err error
-	if requestedEpoch == currentEpoch+1 {
-		epochStartSlot, err = slots.EpochStart(requestedEpoch.Sub(1))
-		if err != nil {
-			return nil, errors.Wrap(err, "Could not obtain epoch's start slot")
-		}
-	} else {
-		epochStartSlot, err = slots.EpochStart(requestedEpoch)
-		if err != nil {
-			return nil, errors.Wrap(err, "Could not obtain epoch's start slot")
-		}
-	}
-	s, err = transition.ProcessSlotsIfPossible(ctx, s, epochStartSlot)
+	root, err := helpers.BlockRootAtSlot(s, dependentRootSlot)
 	if err != nil {
-		return nil, errors.Wrapf(err, "Could not process slots up to %d", epochStartSlot)
+		return nil, errors.Wrap(err, "could not get block root")
 	}
-
-	return s, nil
+	return root, nil
 }
 
 // Logic based on https://hackmd.io/ofFJ5gOmQpu1jjHilHbdQQ

diff --git a/beacon-chain/rpc/eth/validator/validator_test.go b/beacon-chain/rpc/eth/validator/validator_test.go
@@ -85,7 +85,7 @@ func TestGetAttesterDuties(t *testing.T) {
 		State: bs, Root: genesisRoot[:], Slot: &chainSlot,
 	}
 	vs := &Server{
-		HeadFetcher:           chain,
+		StateFetcher:          &testutil.MockFetcher{BeaconState: bs},
 		TimeFetcher:           chain,
 		SyncChecker:           &mockSync.Sync{IsSyncing: false},
 		OptimisticModeFetcher: chain,
@@ -139,55 +139,6 @@ func TestGetAttesterDuties(t *testing.T) {
 		assert.Equal(t, types.CommitteeIndex(110), duty.ValidatorCommitteeIndex)
 	})
 
-	t.Run("Require slot processing", func(t *testing.T) {
-		// We create local variables to not interfere with other tests.
-		// Slot processing might have unexpected side-effects.
-
-		bs, err := transition.GenesisBeaconState(context.Background(), deposits, 0, eth1Data)
-		require.NoError(t, err, "Could not set up genesis state")
-		// Set state to non-epoch start slot.
-		require.NoError(t, bs.SetSlot(5))
-		genesisRoot, err := genesis.Block.HashTreeRoot()
-		require.NoError(t, err, "Could not get signing root")
-		roots := make([][]byte, fieldparams.BlockRootsLength)
-		roots[0] = genesisRoot[:]
-		require.NoError(t, bs.SetBlockRoots(roots))
-
-		pubKeys := make([][]byte, len(deposits))
-		indices := make([]uint64, len(deposits))
-		for i := 0; i < len(deposits); i++ {
-			pubKeys[i] = deposits[i].Data.PublicKey
-			indices[i] = uint64(i)
-		}
-		chainSlot := params.BeaconConfig().SlotsPerEpoch.Mul(2)
-		chain := &mockChain.ChainService{
-			State: bs, Root: genesisRoot[:], Slot: &chainSlot,
-		}
-		vs := &Server{
-			HeadFetcher:           chain,
-			TimeFetcher:           chain,
-			OptimisticModeFetcher: chain,
-			SyncChecker:           &mockSync.Sync{IsSyncing: false},
-		}
-
-		req := &ethpbv1.AttesterDutiesRequest{
-			Epoch: 2,
-			Index: []types.ValidatorIndex{0},
-		}
-		resp, err := vs.GetAttesterDuties(ctx, req)
-		require.NoError(t, err)
-		assert.DeepEqual(t, bs.BlockRoots()[31], resp.DependentRoot)
-		require.Equal(t, 1, len(resp.Data))
-		duty := resp.Data[0]
-		assert.Equal(t, types.CommitteeIndex(1), duty.CommitteeIndex)
-		assert.Equal(t, types.Slot(86), duty.Slot)
-		assert.Equal(t, types.ValidatorIndex(0), duty.ValidatorIndex)
-		assert.DeepEqual(t, pubKeys[0], duty.Pubkey)
-		assert.Equal(t, uint64(128), duty.CommitteeLength)
-		assert.Equal(t, uint64(4), duty.CommitteesAtSlot)
-		assert.Equal(t, types.CommitteeIndex(44), duty.ValidatorCommitteeIndex)
-	})
-
 	t.Run("Epoch out of bound", func(t *testing.T) {
 		currentEpoch := slots.ToEpoch(bs.Slot())
 		req := &ethpbv1.AttesterDutiesRequest{
@@ -234,7 +185,7 @@ func TestGetAttesterDuties(t *testing.T) {
 			State: bs, Root: genesisRoot[:], Slot: &chainSlot, Optimistic: true,
 		}
 		vs := &Server{
-			HeadFetcher:           chain,
+			StateFetcher:          &testutil.MockFetcher{BeaconState: bs},
 			TimeFetcher:           chain,
 			OptimisticModeFetcher: chain,
 			SyncChecker:           &mockSync.Sync{IsSyncing: false},
@@ -271,35 +222,35 @@ func TestGetProposerDuties(t *testing.T) {
 	require.NoError(t, err)
 	eth1Data, err := util.DeterministicEth1Data(len(deposits))
 	require.NoError(t, err)
-	bs, err := transition.GenesisBeaconState(context.Background(), deposits, 0, eth1Data)
-	require.NoError(t, err, "Could not set up genesis state")
-	// Set state to non-epoch start slot.
-	require.NoError(t, bs.SetSlot(5))
 	genesisRoot, err := genesis.Block.HashTreeRoot()
 	require.NoError(t, err, "Could not get signing root")
 	roots := make([][]byte, fieldparams.BlockRootsLength)
 	roots[0] = genesisRoot[:]
-	require.NoError(t, bs.SetBlockRoots(roots))
 	db := dbutil.SetupDB(t)
 
 	pubKeys := make([][]byte, len(deposits))
 	for i := 0; i < len(deposits); i++ {
 		pubKeys[i] = deposits[i].Data.PublicKey
 	}
 
-	chainSlot := types.Slot(0)
-	chain := &mockChain.ChainService{
-		State: bs, Root: genesisRoot[:], Slot: &chainSlot,
-	}
-	vs := &Server{
-		HeadFetcher:            chain,
-		TimeFetcher:            chain,
-		OptimisticModeFetcher:  chain,
-		SyncChecker:            &mockSync.Sync{IsSyncing: false},
-		ProposerSlotIndexCache: cache.NewProposerPayloadIDsCache(),
-	}
-
 	t.Run("Ok", func(t *testing.T) {
+		bs, err := transition.GenesisBeaconState(context.Background(), deposits, 0, eth1Data)
+		require.NoError(t, err, "Could not set up genesis state")
+		require.NoError(t, bs.SetSlot(params.BeaconConfig().SlotsPerEpoch))
+		require.NoError(t, bs.SetBlockRoots(roots))
+		chainSlot := types.Slot(0)
+		chain := &mockChain.ChainService{
+			State: bs, Root: genesisRoot[:], Slot: &chainSlot,
+		}
+		vs := &Server{
+			StateFetcher:           &testutil.MockFetcher{BeaconState: bs},
+			HeadFetcher:            chain,
+			TimeFetcher:            chain,
+			OptimisticModeFetcher:  chain,
+			SyncChecker:            &mockSync.Sync{IsSyncing: false},
+			ProposerSlotIndexCache: cache.NewProposerPayloadIDsCache(),
+		}
+
 		req := &ethpbv1.ProposerDutiesRequest{
 			Epoch: 0,
 		}
@@ -323,14 +274,31 @@ func TestGetProposerDuties(t *testing.T) {
 	})
 
 	t.Run("Prune payload ID cache ok", func(t *testing.T) {
+		bs, err := transition.GenesisBeaconState(context.Background(), deposits, 0, eth1Data)
+		require.NoError(t, err, "Could not set up genesis state")
+		require.NoError(t, bs.SetSlot(params.BeaconConfig().SlotsPerEpoch))
+		require.NoError(t, bs.SetBlockRoots(roots))
+		chainSlot := types.Slot(0)
+		chain := &mockChain.ChainService{
+			State: bs, Root: genesisRoot[:], Slot: &chainSlot,
+		}
+		vs := &Server{
+			StateFetcher:           &testutil.MockFetcher{BeaconState: bs},
+			HeadFetcher:            chain,
+			TimeFetcher:            chain,
+			OptimisticModeFetcher:  chain,
+			SyncChecker:            &mockSync.Sync{IsSyncing: false},
+			ProposerSlotIndexCache: cache.NewProposerPayloadIDsCache(),
+		}
+
 		req := &ethpbv1.ProposerDutiesRequest{
 			Epoch: 1,
 		}
 		vs.ProposerSlotIndexCache.SetProposerAndPayloadIDs(1, 1, [8]byte{1}, [32]byte{2})
 		vs.ProposerSlotIndexCache.SetProposerAndPayloadIDs(31, 2, [8]byte{2}, [32]byte{3})
 		vs.ProposerSlotIndexCache.SetProposerAndPayloadIDs(32, 4309, [8]byte{3}, [32]byte{4})
 
-		_, err := vs.GetProposerDuties(ctx, req)
+		_, err = vs.GetProposerDuties(ctx, req)
 		require.NoError(t, err)
 
 		vid, _, has := vs.ProposerSlotIndexCache.GetProposerPayloadIDs(1, [32]byte{})
@@ -344,68 +312,40 @@ func TestGetProposerDuties(t *testing.T) {
 		require.Equal(t, types.ValidatorIndex(4309), vid)
 	})
 
-	t.Run("Require slot processing", func(t *testing.T) {
-		// We create local variables to not interfere with other tests.
-		// Slot processing might have unexpected side-effects.
-
+	t.Run("Epoch out of bound", func(t *testing.T) {
 		bs, err := transition.GenesisBeaconState(context.Background(), deposits, 0, eth1Data)
 		require.NoError(t, err, "Could not set up genesis state")
 		// Set state to non-epoch start slot.
 		require.NoError(t, bs.SetSlot(5))
-		genesisRoot, err := genesis.Block.HashTreeRoot()
-		require.NoError(t, err, "Could not get signing root")
-		roots := make([][]byte, fieldparams.BlockRootsLength)
-		roots[0] = genesisRoot[:]
 		require.NoError(t, bs.SetBlockRoots(roots))
-
-		pubKeys := make([][]byte, len(deposits))
-		indices := make([]uint64, len(deposits))
-		for i := 0; i < len(deposits); i++ {
-			pubKeys[i] = deposits[i].Data.PublicKey
-			indices[i] = uint64(i)
-		}
-		chainSlot := params.BeaconConfig().SlotsPerEpoch.Mul(2)
+		chainSlot := types.Slot(0)
 		chain := &mockChain.ChainService{
 			State: bs, Root: genesisRoot[:], Slot: &chainSlot,
 		}
 		vs := &Server{
+			StateFetcher:           &testutil.MockFetcher{BeaconState: bs},
 			HeadFetcher:            chain,
 			TimeFetcher:            chain,
 			OptimisticModeFetcher:  chain,
 			SyncChecker:            &mockSync.Sync{IsSyncing: false},
 			ProposerSlotIndexCache: cache.NewProposerPayloadIDsCache(),
 		}
 
-		req := &ethpbv1.ProposerDutiesRequest{
-			Epoch: 2,
-		}
-		resp, err := vs.GetProposerDuties(ctx, req)
-		require.NoError(t, err)
-		assert.DeepEqual(t, bs.BlockRoots()[31], resp.DependentRoot)
-		assert.Equal(t, 32, len(resp.Data))
-		// We expect a proposer duty for slot 74.
-		var expectedDuty *ethpbv1.ProposerDuty
-		for _, duty := range resp.Data {
-			if duty.Slot == 74 {
-				expectedDuty = duty
-			}
-		}
-		require.NotNil(t, expectedDuty, "Expected duty for slot 74 not found")
-		assert.Equal(t, types.ValidatorIndex(11741), expectedDuty.ValidatorIndex)
-		assert.DeepEqual(t, pubKeys[11741], expectedDuty.Pubkey)
-	})
-
-	t.Run("Epoch out of bound", func(t *testing.T) {
 		currentEpoch := slots.ToEpoch(bs.Slot())
 		req := &ethpbv1.ProposerDutiesRequest{
 			Epoch: currentEpoch + 2,
 		}
-		_, err := vs.GetProposerDuties(ctx, req)
+		_, err = vs.GetProposerDuties(ctx, req)
 		require.NotNil(t, err)
 		assert.ErrorContains(t, fmt.Sprintf("Request epoch %d can not be greater than next epoch %d", currentEpoch+2, currentEpoch+1), err)
 	})
 
 	t.Run("execution optimistic", func(t *testing.T) {
+		bs, err := transition.GenesisBeaconState(context.Background(), deposits, 0, eth1Data)
+		require.NoError(t, err, "Could not set up genesis state")
+		// Set state to non-epoch start slot.
+		require.NoError(t, bs.SetSlot(5))
+		require.NoError(t, bs.SetBlockRoots(roots))
 		parentRoot := [32]byte{'a'}
 		blk := util.NewBeaconBlock()
 		blk.Block.ParentRoot = parentRoot[:]
@@ -420,6 +360,7 @@ func TestGetProposerDuties(t *testing.T) {
 			State: bs, Root: genesisRoot[:], Slot: &chainSlot, Optimistic: true,
 		}
 		vs := &Server{
+			StateFetcher:           &testutil.MockFetcher{BeaconState: bs},
 			HeadFetcher:            chain,
 			TimeFetcher:            chain,
 			OptimisticModeFetcher:  chain,