Correct duties calculation for old slots (#11723)

* initial code

* update proposalDependentRoot

* convert method to function

* decouple tests

Co-authored-by: terencechain <terence@prysmaticlabs.com>
This commit is contained in:
Radosław Kapka 2022-12-14 01:33:30 +01:00 committed by GitHub
parent 5ed52b1e44
commit 9efc89f993
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 60 additions and 157 deletions

View File

@ -13,7 +13,6 @@ go_library(
"//beacon-chain/builder:go_default_library",
"//beacon-chain/cache:go_default_library",
"//beacon-chain/core/helpers:go_default_library",
"//beacon-chain/core/transition:go_default_library",
"//beacon-chain/db/kv:go_default_library",
"//beacon-chain/operations/attestations:go_default_library",
"//beacon-chain/operations/synccommittee:go_default_library",

View File

@ -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

View File

@ -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,15 +222,10 @@ 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))
@ -287,19 +233,24 @@ func TestGetProposerDuties(t *testing.T) {
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,6 +274,23 @@ 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,
}
@ -330,7 +298,7 @@ func TestGetProposerDuties(t *testing.T) {
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,31 +312,18 @@ 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,
@ -376,36 +331,21 @@ func TestGetProposerDuties(t *testing.T) {
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,