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

New prysm get block RPC #11834

Merged
merged 43 commits into from
Jan 13, 2023
Merged
Show file tree
Hide file tree
Changes from 19 commits
Commits
Show all changes
43 commits
Select commit Hold shift + click to select a range
5bfb952
Revamp proposer rpc
terencechain Dec 27, 2022
b557ee1
Add canUseBuilder helper method
terencechain Dec 28, 2022
f5aad1d
Merge branch 'can-use-builder' into new-proposer-rpc
terencechain Dec 28, 2022
ab8b0d3
Can build. Need to fix minimal setting
terencechain Dec 28, 2022
b72323f
Fix build.bazel for v1alpha1 rpcs
terencechain Dec 28, 2022
6d9df33
Merge branch 'fix-proposer-rpc-build-file' into new-proposer-rpc
terencechain Dec 28, 2022
a14b667
Rm unused code
terencechain Dec 28, 2022
a8986b9
Rm old tests
terencechain Dec 30, 2022
2851145
Add back tests
terencechain Dec 30, 2022
a48b75c
Sync with develop
terencechain Dec 30, 2022
9a0aa6e
Fix more tests
terencechain Dec 30, 2022
e298d35
Gazelle
terencechain Dec 30, 2022
aa4ac54
Merge branch 'develop' of github.com:prysmaticlabs/prysm into new-pro…
terencechain Jan 4, 2023
144b800
Add capella test
terencechain Jan 4, 2023
a29bef8
Use metric
terencechain Jan 4, 2023
a1e86bf
Use timeout
terencechain Jan 4, 2023
2e038fa
Go fmt
terencechain Jan 4, 2023
933e920
Disable transition cache
terencechain Jan 4, 2023
8f65d2c
Merge branch 'develop' into new-proposer-rpc
terencechain Jan 4, 2023
c84b525
Refactor fork based helpers
terencechain Jan 5, 2023
7ad424a
Merge branch 'new-proposer-rpc' of github.com:prysmaticlabs/prysm int…
terencechain Jan 5, 2023
45b31b5
Merge branch 'develop' into new-proposer-rpc
terencechain Jan 5, 2023
f380656
Fix return
terencechain Jan 5, 2023
edc24be
Merge branch 'new-proposer-rpc' of github.com:prysmaticlabs/prysm int…
terencechain Jan 5, 2023
4891145
Fix test
terencechain Jan 5, 2023
4756bbe
Potuz feedback
terencechain Jan 6, 2023
d608353
Fix bls to exec data
terencechain Jan 6, 2023
3ad693e
Move deposit and att outside of else block
terencechain Jan 6, 2023
daa1104
Fix err msg
terencechain Jan 6, 2023
d0d5cde
Rm elses
terencechain Jan 6, 2023
a40c2f9
Better reconstruct error msg when EL client is syncing
terencechain Jan 8, 2023
13baabf
Merge branch 'better-reconstruct-msg' into new-proposer-rpc
terencechain Jan 8, 2023
dd883df
Tested and worked
terencechain Jan 8, 2023
dd591bb
Potuz feedback
terencechain Jan 11, 2023
9361785
Merge branch 'develop' of github.com:prysmaticlabs/prysm into new-pro…
terencechain Jan 11, 2023
e2c59cc
Rm
terencechain Jan 11, 2023
8291ed8
Fix test
terencechain Jan 12, 2023
455197d
Merge branch 'develop' into new-proposer-rpc
terencechain Jan 12, 2023
44b6099
Sync aggregate empty case
terencechain Jan 13, 2023
a2f1137
Merge branch 'develop' into new-proposer-rpc
terencechain Jan 13, 2023
ac95af3
Dont set it
terencechain Jan 13, 2023
142cf8d
Merge branch 'new-proposer-rpc' of github.com:prysmaticlabs/prysm int…
terencechain Jan 13, 2023
575e523
Refactor to set empty within the error
terencechain Jan 13, 2023
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
1 change: 1 addition & 0 deletions beacon-chain/execution/testing/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ go_library(
"//encoding/bytesutil:go_default_library",
"//proto/engine/v1:go_default_library",
"//proto/prysm/v1alpha1:go_default_library",
"//time/slots:go_default_library",
"@com_github_ethereum_go_ethereum//accounts/abi/bind/backends:go_default_library",
"@com_github_ethereum_go_ethereum//common:go_default_library",
"@com_github_ethereum_go_ethereum//common/hexutil:go_default_library",
Expand Down
7 changes: 6 additions & 1 deletion beacon-chain/execution/testing/mock_engine_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import (
types "github.com/prysmaticlabs/prysm/v3/consensus-types/primitives"
"github.com/prysmaticlabs/prysm/v3/encoding/bytesutil"
pb "github.com/prysmaticlabs/prysm/v3/proto/engine/v1"
"github.com/prysmaticlabs/prysm/v3/time/slots"
)

// EngineClient --
Expand All @@ -23,6 +24,7 @@ type EngineClient struct {
PayloadIDBytes *pb.PayloadIDBytes
ForkChoiceUpdatedResp []byte
ExecutionPayload *pb.ExecutionPayload
ExecutionPayloadCapella *pb.ExecutionPayloadCapella
ExecutionBlock *pb.ExecutionBlock
Err error
ErrLatestExecBlock error
Expand Down Expand Up @@ -54,7 +56,10 @@ func (e *EngineClient) ForkchoiceUpdated(
}

// GetPayload --
func (e *EngineClient) GetPayload(_ context.Context, _ [8]byte, _ types.Slot) (interfaces.ExecutionData, error) {
func (e *EngineClient) GetPayload(_ context.Context, _ [8]byte, s types.Slot) (interfaces.ExecutionData, error) {
if slots.ToEpoch(s) >= params.BeaconConfig().CapellaForkEpoch {
return blocks.WrappedExecutionPayloadCapella(e.ExecutionPayloadCapella)
}
p, err := blocks.WrappedExecutionPayload(e.ExecutionPayload)
if err != nil {
return nil, err
Expand Down
9 changes: 1 addition & 8 deletions beacon-chain/rpc/eth/validator/validator.go
Original file line number Diff line number Diff line change
Expand Up @@ -500,15 +500,8 @@ func (vs *Server) ProduceBlindedBlock(ctx context.Context, req *ethpbv1.ProduceB
if optimistic {
return nil, status.Errorf(codes.Unavailable, "The node is currently optimistic and cannot serve validators")
}
altairBlk, err := vs.V1Alpha1Server.BuildAltairBeaconBlock(ctx, v1alpha1req)
b, err := vs.V1Alpha1Server.GetBeaconBlock(ctx, v1alpha1req)
if err != nil {
return nil, status.Errorf(codes.Internal, "Could not prepare beacon block: %v", err)
}
ok, b, err := vs.V1Alpha1Server.GetAndBuildBlindBlock(ctx, altairBlk)
if err != nil {
return nil, status.Errorf(codes.Internal, "Could not prepare blind beacon block: %v", err)
}
if !ok {
return nil, status.Error(codes.Unavailable, "Builder is not available due to miss-config or circuit breaker")
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this error right now?

Copy link
Member Author

Choose a reason for hiding this comment

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

This is a beacon API for retrieving blind blocks (only). By the beacon API definition, you must return a valid blind block or fail. Very different than our current implementation used by Prysm API

Copy link
Contributor

Choose a reason for hiding this comment

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

I mean are you sure that the only failures are miss-config or circuit breaker in this path?

Copy link
Member Author

Choose a reason for hiding this comment

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

The error msg is wrong. I'll update

}
blk, err := migration.V1Alpha1BeaconBlockBlindedBellatrixToV2Blinded(b.GetBlindedBellatrix())
Expand Down
54 changes: 29 additions & 25 deletions beacon-chain/rpc/eth/validator/validator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -689,18 +689,20 @@ func TestProduceBlockV2(t *testing.T) {
mockChainService := &mockChain.ChainService{State: beaconState, Root: parentRoot[:]}
mockExecutionChain := &mockExecution.Chain{}
v1Alpha1Server := &v1alpha1validator.Server{
HeadFetcher: mockChainService,
SyncChecker: &mockSync.Sync{IsSyncing: false},
BlockReceiver: mockChainService,
HeadUpdater: mockChainService,
ChainStartFetcher: mockExecutionChain,
Eth1InfoFetcher: mockExecutionChain,
Eth1BlockFetcher: mockExecutionChain,
MockEth1Votes: true,
AttPool: attestations.NewPool(),
SlashingsPool: slashings.NewPool(),
ExitPool: voluntaryexits.NewPool(),
StateGen: stategen.New(db, doublylinkedtree.New()),
HeadFetcher: mockChainService,
SyncChecker: &mockSync.Sync{IsSyncing: false},
BlockReceiver: mockChainService,
HeadUpdater: mockChainService,
ChainStartFetcher: mockExecutionChain,
Eth1InfoFetcher: mockExecutionChain,
Eth1BlockFetcher: mockExecutionChain,
MockEth1Votes: true,
AttPool: attestations.NewPool(),
SlashingsPool: slashings.NewPool(),
ExitPool: voluntaryexits.NewPool(),
StateGen: stategen.New(db, doublylinkedtree.New()),
TimeFetcher: mockChainService,
OptimisticModeFetcher: mockChainService,
}

proposerSlashings := make([]*ethpbalpha.ProposerSlashing, params.BeaconConfig().MaxProposerSlashings)
Expand Down Expand Up @@ -797,19 +799,21 @@ func TestProduceBlockV2(t *testing.T) {
mochChainService := &mockChain.ChainService{State: beaconState, Root: parentRoot[:]}
mockExecutionChain := &mockExecution.Chain{}
v1Alpha1Server := &v1alpha1validator.Server{
HeadFetcher: mochChainService,
SyncChecker: &mockSync.Sync{IsSyncing: false},
BlockReceiver: mochChainService,
HeadUpdater: mochChainService,
ChainStartFetcher: mockExecutionChain,
Eth1InfoFetcher: mockExecutionChain,
Eth1BlockFetcher: mockExecutionChain,
MockEth1Votes: true,
AttPool: attestations.NewPool(),
SlashingsPool: slashings.NewPool(),
ExitPool: voluntaryexits.NewPool(),
StateGen: stategen.New(db, doublylinkedtree.New()),
SyncCommitteePool: synccommittee.NewStore(),
HeadFetcher: mochChainService,
SyncChecker: &mockSync.Sync{IsSyncing: false},
BlockReceiver: mochChainService,
HeadUpdater: mochChainService,
ChainStartFetcher: mockExecutionChain,
Eth1InfoFetcher: mockExecutionChain,
Eth1BlockFetcher: mockExecutionChain,
MockEth1Votes: true,
AttPool: attestations.NewPool(),
SlashingsPool: slashings.NewPool(),
ExitPool: voluntaryexits.NewPool(),
StateGen: stategen.New(db, doublylinkedtree.New()),
SyncCommitteePool: synccommittee.NewStore(),
TimeFetcher: mochChainService,
OptimisticModeFetcher: mochChainService,
}

proposerSlashings := make([]*ethpbalpha.ProposerSlashing, params.BeaconConfig().MaxProposerSlashings)
Expand Down
4 changes: 2 additions & 2 deletions beacon-chain/rpc/prysm/v1alpha1/validator/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ go_library(
"proposer_eth1data.go",
"proposer_execution_payload.go",
"proposer_exits.go",
"proposer_phase0.go",
"proposer_slashings.go",
"proposer_sync_aggregate.go",
"server.go",
Expand All @@ -44,12 +43,12 @@ go_library(
"//beacon-chain/core/signing:go_default_library",
"//beacon-chain/core/time:go_default_library",
"//beacon-chain/core/transition:go_default_library",
"//beacon-chain/core/transition/interop:go_default_library",
"//beacon-chain/core/validators:go_default_library",
"//beacon-chain/db:go_default_library",
"//beacon-chain/db/kv:go_default_library",
"//beacon-chain/execution:go_default_library",
"//beacon-chain/operations/attestations:go_default_library",
"//beacon-chain/operations/blstoexec:go_default_library",
"//beacon-chain/operations/slashings:go_default_library",
"//beacon-chain/operations/synccommittee:go_default_library",
"//beacon-chain/operations/voluntaryexits:go_default_library",
Expand Down Expand Up @@ -125,6 +124,7 @@ common_deps = [
"//beacon-chain/operations/synccommittee:go_default_library",
"//beacon-chain/operations/voluntaryexits:go_default_library",
"//beacon-chain/p2p/testing:go_default_library",
"//beacon-chain/rpc/testutil:go_default_library",
"//beacon-chain/state:go_default_library",
"//beacon-chain/state/state-native:go_default_library",
"//beacon-chain/state/stategen:go_default_library",
Expand Down
153 changes: 141 additions & 12 deletions beacon-chain/rpc/prysm/v1alpha1/validator/proposer.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,10 @@ import (
"github.com/prysmaticlabs/prysm/v3/beacon-chain/builder"
"github.com/prysmaticlabs/prysm/v3/beacon-chain/core/feed"
blockfeed "github.com/prysmaticlabs/prysm/v3/beacon-chain/core/feed/block"
"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"
fieldparams "github.com/prysmaticlabs/prysm/v3/config/fieldparams"
"github.com/prysmaticlabs/prysm/v3/config/params"
"github.com/prysmaticlabs/prysm/v3/consensus-types/blocks"
"github.com/prysmaticlabs/prysm/v3/consensus-types/interfaces"
Expand All @@ -41,26 +43,153 @@ func (vs *Server) GetBeaconBlock(ctx context.Context, req *ethpb.BlockRequest) (
ctx, span := trace.StartSpan(ctx, "ProposerServer.GetBeaconBlock")
defer span.End()
span.AddAttributes(trace.Int64Attribute("slot", int64(req.Slot)))
if slots.ToEpoch(req.Slot) < params.BeaconConfig().AltairForkEpoch {
blk, err := vs.getPhase0BeaconBlock(ctx, req)

// A syncing validator should not produce a block.
if vs.SyncChecker.Syncing() {
return nil, status.Error(codes.Unavailable, "Syncing to latest head, not ready to respond")
}

// An optimistic validator MUST NOT produce a block (i.e., sign across the DOMAIN_BEACON_PROPOSER domain).
if err := vs.optimisticStatus(ctx); err != nil {
return nil, status.Errorf(codes.Unavailable, "Validator is not ready to propose: %v", err)
}

sBlk, err := getEmptyBlock(req.Slot)
if err != nil {
return nil, status.Errorf(codes.Internal, "Could not prepare block: %v", err)
}

parentRoot, err := vs.HeadFetcher.HeadRoot(ctx)
if err != nil {
return nil, status.Errorf(codes.Internal, "Could not get head root: %v", err)
}
head, err := vs.HeadFetcher.HeadState(ctx)
if err != nil {
return nil, status.Errorf(codes.Internal, "Could not get head state: %v", err)
}
head, err = transition.ProcessSlotsUsingNextSlotCache(ctx, head, parentRoot, req.Slot)
if err != nil {
return nil, status.Errorf(codes.Internal, "Could not process slots up to %d: %v", req.Slot, err)
}

blk := sBlk.Block()
// Set slot, graffiti, randao reveal, and parent root.
blk.SetSlot(req.Slot)
blk.Body().SetGraffiti(req.Graffiti)
blk.Body().SetRandaoReveal(req.RandaoReveal)
blk.SetParentRoot(parentRoot)

// Set eth1 data.
eth1Data, err := vs.eth1DataMajorityVote(ctx, head)
if err != nil {
log.WithError(err).Error("Could not get eth1data")
potuz marked this conversation as resolved.
Show resolved Hide resolved
} else {
blk.Body().SetEth1Data(eth1Data)

// Set deposit and attestation.
deposits, atts, err := vs.packDepositsAndAttestations(ctx, head, eth1Data) // TODO: split attestations and deposits
if err != nil {
log.WithError(err).Error("Could not pack deposits and attestations")
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here, do we need to set the empty deposits and attestations here or the marshaller takes care of it automatically?

Copy link
Member Author

@terencechain terencechain Jan 6, 2023

Choose a reason for hiding this comment

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

I think this one is fine since those are not vectors but I added anyway

} else {
blk.Body().SetDeposits(deposits)
blk.Body().SetAttestations(atts)
}
}

// Set proposer index
idx, err := helpers.BeaconProposerIndex(ctx, head)
if err != nil {
return nil, fmt.Errorf("could not calculate proposer index %v", err)
}
blk.SetProposerIndex(idx)

// Set slashings
validProposerSlashings, validAttSlashings := vs.getSlashings(ctx, head)
blk.Body().SetProposerSlashings(validProposerSlashings)
blk.Body().SetAttesterSlashings(validAttSlashings)

// Set exits
blk.Body().SetVoluntaryExits(vs.getExits(head, req.Slot))

// Set sync aggregate. New in Altair.
if req.Slot > 0 && slots.ToEpoch(req.Slot) >= params.BeaconConfig().AltairForkEpoch {
syncAggregate, err := vs.getSyncAggregate(ctx, req.Slot-1, bytesutil.ToBytes32(parentRoot))
if err != nil {
return nil, status.Errorf(codes.Internal, "Could not fetch phase0 beacon block: %v", err)
log.WithError(err).Error("Could not get sync aggregate")
Copy link
Contributor

Choose a reason for hiding this comment

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

No need to return right?

Copy link
Member Author

Choose a reason for hiding this comment

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

We don't want to return here. There's no reason to miss a block for this error

} else {
if err := blk.Body().SetSyncAggregate(syncAggregate); err != nil {
log.WithError(err).Error("Could not set sync aggregate")
if err := blk.Body().SetSyncAggregate(&ethpb.SyncAggregate{
SyncCommitteeBits: make([]byte, params.BeaconConfig().SyncCommitteeSize),
SyncCommitteeSignature: make([]byte, fieldparams.BLSSignatureLength),
}); err != nil {
return nil, status.Errorf(codes.Internal, "Could not set default sync aggregate: %v", err)
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we return here ? or let it propose?

Copy link
Member Author

Choose a reason for hiding this comment

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

Same as above

}
}
}
return &ethpb.GenericBeaconBlock{Block: &ethpb.GenericBeaconBlock_Phase0{Phase0: blk}}, nil
} else if slots.ToEpoch(req.Slot) < params.BeaconConfig().BellatrixForkEpoch {
blk, err := vs.getAltairBeaconBlock(ctx, req)
}

// Set execution data. New in Bellatrix
if slots.ToEpoch(req.Slot) >= params.BeaconConfig().BellatrixForkEpoch {
Copy link
Contributor

Choose a reason for hiding this comment

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

All these nested conditionals and control variables make me nervous. I would prefer we have a small function that does these different tasks and has almost full coverage. It should help with reducing nesting. It could be cleaner if we have small functions that apply altair things, apply bellatrix things, and apply capella things. Each one thoroughly tested and should be small (less than 50 loc)

Copy link
Member Author

Choose a reason for hiding this comment

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

Done, I'll be adding tests over this later

fallBackToLocal := true
canUseBuilder, err := vs.canUseBuilder(ctx, req.Slot, idx)
if err != nil {
return nil, status.Errorf(codes.Internal, "Could not fetch Altair beacon block: %v", err)
log.WithError(err).Warn("Proposer: failed to check if builder can be used")
} else if canUseBuilder {
h, err := vs.getPayloadHeaderFromBuilder(ctx, req.Slot, idx)
if err != nil {
builderGetPayloadMissCount.Inc()
log.WithError(err).Warn("Proposer: failed to get payload header from builder")
} else {
blk.SetBlinded(true)
if err := blk.Body().SetExecution(h); err != nil {
log.WithError(err).Warn("Proposer: failed to set execution payload")
} else {
fallBackToLocal = false
}
}
}
if fallBackToLocal {
executionData, err := vs.getExecutionPayload(ctx, req.Slot, idx, bytesutil.ToBytes32(parentRoot), head)
if err != nil {
return nil, status.Errorf(codes.Internal, "Could not get execution payload: %v", err)
}
if err := blk.Body().SetExecution(executionData); err != nil {
return nil, status.Errorf(codes.Internal, "Could not set execution payload: %v", err)
}
}
return &ethpb.GenericBeaconBlock{Block: &ethpb.GenericBeaconBlock_Altair{Altair: blk}}, nil
}

// An optimistic validator MUST NOT produce a block (i.e., sign across the DOMAIN_BEACON_PROPOSER domain).
if err := vs.optimisticStatus(ctx); err != nil {
return nil, err
// Set bls to execution change. New in Capella
if slots.ToEpoch(req.Slot) >= params.BeaconConfig().CapellaForkEpoch {
changes, err := vs.BLSChangesPool.BLSToExecChangesForInclusion(head)
if err != nil {
log.WithError(err).Error("Could not get bls to execution changes")
} else {
if err := blk.Body().SetBLSToExecutionChanges(changes); err != nil {
log.WithError(err).Error("Could not set bls to execution changes")
}
}
}

return vs.getBellatrixBeaconBlock(ctx, req)
sr, err := vs.computeStateRoot(ctx, sBlk)
if err != nil {
return nil, status.Errorf(codes.Internal, "Could not compute state root: %v", err)
}
blk.SetStateRoot(sr)

pb, err := blk.Proto()
if err != nil {
return nil, status.Errorf(codes.Internal, "Could not convert block to proto: %v", err)
}
if slots.ToEpoch(req.Slot) >= params.BeaconConfig().CapellaForkEpoch {
potuz marked this conversation as resolved.
Show resolved Hide resolved
return &ethpb.GenericBeaconBlock{Block: &ethpb.GenericBeaconBlock_Capella{Capella: pb.(*ethpb.BeaconBlockCapella)}}, nil
} else if slots.ToEpoch(req.Slot) >= params.BeaconConfig().BellatrixForkEpoch {
return &ethpb.GenericBeaconBlock{Block: &ethpb.GenericBeaconBlock_Bellatrix{Bellatrix: pb.(*ethpb.BeaconBlockBellatrix)}}, nil
} else if slots.ToEpoch(req.Slot) >= params.BeaconConfig().AltairForkEpoch {
return &ethpb.GenericBeaconBlock{Block: &ethpb.GenericBeaconBlock_Altair{Altair: pb.(*ethpb.BeaconBlockAltair)}}, nil
}
return &ethpb.GenericBeaconBlock{Block: &ethpb.GenericBeaconBlock_Phase0{Phase0: pb.(*ethpb.BeaconBlock)}}, nil
}

// ProposeBeaconBlock is called by a proposer during its assigned slot to create a block in an attempt
Expand Down
Loading