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

EIP-7549: validator client #14158

Merged
merged 9 commits into from
Jul 11, 2024
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
1 change: 1 addition & 0 deletions beacon-chain/rpc/prysm/v1alpha1/validator/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@ go_library(
"//proto/engine/v1:go_default_library",
"//proto/eth/v1:go_default_library",
"//proto/prysm/v1alpha1:go_default_library",
"//proto/prysm/v1alpha1/attestation:go_default_library",
"//proto/prysm/v1alpha1/attestation/aggregation:go_default_library",
"//proto/prysm/v1alpha1/attestation/aggregation/attestations:go_default_library",
"//proto/prysm/v1alpha1/attestation/aggregation/sync_contribution:go_default_library",
Expand Down
100 changes: 71 additions & 29 deletions beacon-chain/rpc/prysm/v1alpha1/validator/attester.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,54 +39,59 @@ func (vs *Server) GetAttestationData(ctx context.Context, req *ethpb.Attestation
}

// ProposeAttestation is a function called by an attester to vote
// on a block via an attestation object as defined in the Ethereum Serenity specification.
// on a block via an attestation object as defined in the Ethereum specification.
func (vs *Server) ProposeAttestation(ctx context.Context, att *ethpb.Attestation) (*ethpb.AttestResponse, error) {
ctx, span := trace.StartSpan(ctx, "AttesterServer.ProposeAttestation")
defer span.End()

if _, err := bls.SignatureFromBytes(att.Signature); err != nil {
return nil, status.Error(codes.InvalidArgument, "Incorrect attestation signature")
}

root, err := att.Data.HashTreeRoot()
resp, err := vs.proposeAtt(ctx, att, att.GetData().CommitteeIndex)
if err != nil {
return nil, status.Errorf(codes.Internal, "Could not tree hash attestation: %v", err)
return nil, err
}

// Broadcast the unaggregated attestation on a feed to notify other services in the beacon node
// of a received unaggregated attestation.
vs.OperationNotifier.OperationFeed().Send(&feed.Event{
Type: operation.UnaggregatedAttReceived,
Data: &operation.UnAggregatedAttReceivedData{
Attestation: att,
},
})
go func() {
attCopy := ethpb.CopyAttestation(att)
if err := vs.AttPool.SaveUnaggregatedAttestation(attCopy); err != nil {
log.WithError(err).Error("Could not save unaggregated attestation")
return
}
}()

// Determine subnet to broadcast attestation to
wantedEpoch := slots.ToEpoch(att.Data.Slot)
vals, err := vs.HeadFetcher.HeadValidatorsIndices(ctx, wantedEpoch)
if err != nil {
return nil, err
return resp, nil
}

// ProposeAttestationElectra is a function called by an attester to vote
// on a block via an attestation object as defined in the Ethereum specification.
func (vs *Server) ProposeAttestationElectra(ctx context.Context, att *ethpb.AttestationElectra) (*ethpb.AttestResponse, error) {
ctx, span := trace.StartSpan(ctx, "AttesterServer.ProposeAttestationElectra")
defer span.End()

if att.GetData().CommitteeIndex != 0 {
prestonvanloon marked this conversation as resolved.
Show resolved Hide resolved
return nil, status.Errorf(codes.InvalidArgument, "Committee index must be set to 0")
}
committeeIndices := helpers.CommitteeIndices(att.CommitteeBits)
if len(committeeIndices) == 0 {
return nil, status.Errorf(codes.InvalidArgument, "Committee bits has no bit set")
}
if len(committeeIndices) > 1 {
return nil, status.Errorf(codes.InvalidArgument, "Committee bits has more than one bit set")
}
subnet := helpers.ComputeSubnetFromCommitteeAndSlot(uint64(len(vals)), att.Data.CommitteeIndex, att.Data.Slot)

// Broadcast the new attestation to the network.
if err := vs.P2P.BroadcastAttestation(ctx, subnet, att); err != nil {
return nil, status.Errorf(codes.Internal, "Could not broadcast attestation: %v", err)
resp, err := vs.proposeAtt(ctx, att, committeeIndices[0])
if err != nil {
return nil, err
}

go func() {
ctx = trace.NewContext(context.Background(), trace.FromContext(ctx))
attCopy := ethpb.CopyAttestation(att)
attCopy := ethpb.CopyAttestationElectra(att)
if err := vs.AttPool.SaveUnaggregatedAttestation(attCopy); err != nil {
log.WithError(err).Error("Could not handle attestation in operations service")
log.WithError(err).Error("Could not save unaggregated attestation")
return
}
}()

return &ethpb.AttestResponse{
AttestationDataRoot: root[:],
}, nil
return resp, nil
}

// SubscribeCommitteeSubnets subscribes to the committee ID subnet given subscribe request.
Expand Down Expand Up @@ -136,3 +141,40 @@ func (vs *Server) SubscribeCommitteeSubnets(ctx context.Context, req *ethpb.Comm

return &emptypb.Empty{}, nil
}

func (vs *Server) proposeAtt(ctx context.Context, att ethpb.Att, committee primitives.CommitteeIndex) (*ethpb.AttestResponse, error) {
if _, err := bls.SignatureFromBytes(att.GetSignature()); err != nil {
return nil, status.Error(codes.InvalidArgument, "Incorrect attestation signature")
}

root, err := att.GetData().HashTreeRoot()
if err != nil {
return nil, status.Errorf(codes.Internal, "Could not tree hash attestation: %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.

Suggested change
return nil, status.Errorf(codes.Internal, "Could not tree hash attestation: %v", err)
return nil, status.Errorf(codes.Internal, "Could not get attestation hash tree: %v", err)

}

// Broadcast the unaggregated attestation on a feed to notify other services in the beacon node
// of a received unaggregated attestation.
vs.OperationNotifier.OperationFeed().Send(&feed.Event{
Type: operation.UnaggregatedAttReceived,
Data: &operation.UnAggregatedAttReceivedData{
Attestation: att,
},
})

// Determine subnet to broadcast attestation to
wantedEpoch := slots.ToEpoch(att.GetData().Slot)
vals, err := vs.HeadFetcher.HeadValidatorsIndices(ctx, wantedEpoch)
if err != nil {
return nil, err
}
subnet := helpers.ComputeSubnetFromCommitteeAndSlot(uint64(len(vals)), committee, att.GetData().Slot)

// Broadcast the new attestation to the network.
if err := vs.P2P.BroadcastAttestation(ctx, subnet, att); err != nil {
return nil, status.Errorf(codes.Internal, "Could not broadcast attestation: %v", err)
}

return &ethpb.AttestResponse{
AttestationDataRoot: root[:],
}, nil
}
115 changes: 99 additions & 16 deletions beacon-chain/rpc/prysm/v1alpha1/validator/attester_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ import (
"google.golang.org/protobuf/proto"
)

func TestProposeAttestation_OK(t *testing.T) {
func TestProposeAttestation(t *testing.T) {
attesterServer := &Server{
HeadFetcher: &mock.ChainService{},
P2P: &mockp2p.MockBroadcaster{},
Expand All @@ -53,24 +53,107 @@ func TestProposeAttestation_OK(t *testing.T) {
}
}

state, err := util.NewBeaconState()
require.NoError(t, err)
require.NoError(t, state.SetSlot(params.BeaconConfig().SlotsPerEpoch+1))
require.NoError(t, state.SetValidators(validators))

sk, err := bls.RandKey()
require.NoError(t, err)
sig := sk.Sign([]byte("dummy_test_data"))
req := &ethpb.Attestation{
Signature: sig.Marshal(),
Data: &ethpb.AttestationData{
BeaconBlockRoot: root[:],
Source: &ethpb.Checkpoint{Root: make([]byte, 32)},
Target: &ethpb.Checkpoint{Root: make([]byte, 32)},
},
}
_, err = attesterServer.ProposeAttestation(context.Background(), req)
assert.NoError(t, err)

t.Run("Phase 0", func(t *testing.T) {
state, err := util.NewBeaconState()
require.NoError(t, err)
require.NoError(t, state.SetSlot(params.BeaconConfig().SlotsPerEpoch+1))
require.NoError(t, state.SetValidators(validators))

req := &ethpb.Attestation{
Signature: sig.Marshal(),
Data: &ethpb.AttestationData{
BeaconBlockRoot: root[:],
Source: &ethpb.Checkpoint{Root: make([]byte, 32)},
Target: &ethpb.Checkpoint{Root: make([]byte, 32)},
},
}
_, err = attesterServer.ProposeAttestation(context.Background(), req)
assert.NoError(t, err)
})
t.Run("Electra", func(t *testing.T) {
state, err := util.NewBeaconStateElectra()
require.NoError(t, err)
require.NoError(t, state.SetSlot(params.BeaconConfig().SlotsPerEpoch+1))
require.NoError(t, state.SetValidators(validators))

cb := primitives.NewAttestationCommitteeBits()
cb.SetBitAt(0, true)
req := &ethpb.AttestationElectra{
Signature: sig.Marshal(),
Data: &ethpb.AttestationData{
BeaconBlockRoot: root[:],
Source: &ethpb.Checkpoint{Root: make([]byte, 32)},
Target: &ethpb.Checkpoint{Root: make([]byte, 32)},
},
CommitteeBits: cb,
}
_, err = attesterServer.ProposeAttestationElectra(context.Background(), req)
assert.NoError(t, err)
})
t.Run("Electra - non-zero committee index", func(t *testing.T) {
state, err := util.NewBeaconStateElectra()
require.NoError(t, err)
require.NoError(t, state.SetSlot(params.BeaconConfig().SlotsPerEpoch+1))
require.NoError(t, state.SetValidators(validators))

cb := primitives.NewAttestationCommitteeBits()
cb.SetBitAt(0, true)
req := &ethpb.AttestationElectra{
Signature: sig.Marshal(),
Data: &ethpb.AttestationData{
BeaconBlockRoot: root[:],
Source: &ethpb.Checkpoint{Root: make([]byte, 32)},
Target: &ethpb.Checkpoint{Root: make([]byte, 32)},
CommitteeIndex: 1,
},
CommitteeBits: cb,
}
_, err = attesterServer.ProposeAttestationElectra(context.Background(), req)
assert.ErrorContains(t, "Committee index must be set to 0", err)
})
t.Run("Electra - no committee bit set", func(t *testing.T) {
state, err := util.NewBeaconStateElectra()
require.NoError(t, err)
require.NoError(t, state.SetSlot(params.BeaconConfig().SlotsPerEpoch+1))
require.NoError(t, state.SetValidators(validators))

req := &ethpb.AttestationElectra{
Signature: sig.Marshal(),
Data: &ethpb.AttestationData{
BeaconBlockRoot: root[:],
Source: &ethpb.Checkpoint{Root: make([]byte, 32)},
Target: &ethpb.Checkpoint{Root: make([]byte, 32)},
},
CommitteeBits: primitives.NewAttestationCommitteeBits(),
}
_, err = attesterServer.ProposeAttestationElectra(context.Background(), req)
assert.ErrorContains(t, "Committee bits has no bit set", err)
})
t.Run("Electra - multiple committee bits set", func(t *testing.T) {
state, err := util.NewBeaconStateElectra()
require.NoError(t, err)
require.NoError(t, state.SetSlot(params.BeaconConfig().SlotsPerEpoch+1))
require.NoError(t, state.SetValidators(validators))

cb := primitives.NewAttestationCommitteeBits()
cb.SetBitAt(0, true)
cb.SetBitAt(1, true)
req := &ethpb.AttestationElectra{
Signature: sig.Marshal(),
Data: &ethpb.AttestationData{
BeaconBlockRoot: root[:],
Source: &ethpb.Checkpoint{Root: make([]byte, 32)},
Target: &ethpb.Checkpoint{Root: make([]byte, 32)},
},
CommitteeBits: cb,
}
_, err = attesterServer.ProposeAttestationElectra(context.Background(), req)
assert.ErrorContains(t, "Committee bits has more than one bit set", err)
})
}

func TestProposeAttestation_IncorrectSignature(t *testing.T) {
Expand Down
2 changes: 1 addition & 1 deletion beacon-chain/rpc/prysm/v1alpha1/validator/proposer.go
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,7 @@ func (vs *Server) BuildBlockParallel(ctx context.Context, sBlk interfaces.Signed
sBlk.SetEth1Data(eth1Data)

// Set deposit and attestation.
deposits, atts, err := vs.packDepositsAndAttestations(ctx, head, eth1Data) // TODO: split attestations and deposits
deposits, atts, err := vs.packDepositsAndAttestations(ctx, head, sBlk.Block().Slot(), eth1Data) // TODO: split attestations and deposits
if err != nil {
sBlk.SetDeposits([]*ethpb.Deposit{})
if err := sBlk.SetAttestations([]ethpb.Att{}); err != nil {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,17 @@ import (
"github.com/prysmaticlabs/prysm/v5/config/params"
"github.com/prysmaticlabs/prysm/v5/consensus-types/primitives"
ethpb "github.com/prysmaticlabs/prysm/v5/proto/prysm/v1alpha1"
"github.com/prysmaticlabs/prysm/v5/proto/prysm/v1alpha1/attestation"
"github.com/prysmaticlabs/prysm/v5/proto/prysm/v1alpha1/attestation/aggregation"
attaggregation "github.com/prysmaticlabs/prysm/v5/proto/prysm/v1alpha1/attestation/aggregation/attestations"
"github.com/prysmaticlabs/prysm/v5/runtime/version"
"github.com/prysmaticlabs/prysm/v5/time/slots"
"go.opencensus.io/trace"
)

type proposerAtts []ethpb.Att

func (vs *Server) packAttestations(ctx context.Context, latestState state.BeaconState) ([]ethpb.Att, error) {
func (vs *Server) packAttestations(ctx context.Context, latestState state.BeaconState, blkSlot primitives.Slot) ([]ethpb.Att, error) {
ctx, span := trace.StartSpan(ctx, "ProposerServer.packAttestations")
defer span.End()

Expand All @@ -39,20 +42,37 @@ func (vs *Server) packAttestations(ctx context.Context, latestState state.Beacon
}
atts = append(atts, uAtts...)

postElectra := slots.ToEpoch(blkSlot) >= params.BeaconConfig().ElectraForkEpoch

versionAtts := make([]ethpb.Att, 0, len(atts))
if postElectra {
for _, a := range atts {
if a.Version() == version.Electra {
versionAtts = append(versionAtts, a)
}
}
} else {
for _, a := range atts {
if a.Version() == version.Phase0 {
versionAtts = append(versionAtts, a)
}
}
}
Comment on lines +47 to +60
Copy link
Member

Choose a reason for hiding this comment

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

Can you add tests for this function logic? If it's too complicated, consider moving this filtering logic to its own method and testing that.


// Remove duplicates from both aggregated/unaggregated attestations. This
// prevents inefficient aggregates being created.
atts, err = proposerAtts(atts).dedup()
versionAtts, err = proposerAtts(versionAtts).dedup()
if err != nil {
return nil, err
}

attsByDataRoot := make(map[[32]byte][]ethpb.Att, len(atts))
for _, att := range atts {
attDataRoot, err := att.GetData().HashTreeRoot()
attsByDataRoot := make(map[attestation.Id][]ethpb.Att, len(versionAtts))
for _, att := range versionAtts {
id, err := attestation.NewId(att, attestation.Data)
if err != nil {
return nil, err
return nil, errors.Wrap(err, "could not create attestation ID")
}
attsByDataRoot[attDataRoot] = append(attsByDataRoot[attDataRoot], att)
attsByDataRoot[id] = append(attsByDataRoot[id], att)
}

attsForInclusion := proposerAtts(make([]ethpb.Att, 0))
Expand Down Expand Up @@ -169,8 +189,18 @@ func (a proposerAtts) sortByProfitabilityUsingMaxCover() (proposerAtts, error) {

// limitToMaxAttestations limits attestations to maximum attestations per block.
func (a proposerAtts) limitToMaxAttestations() proposerAtts {
if uint64(len(a)) > params.BeaconConfig().MaxAttestations {
return a[:params.BeaconConfig().MaxAttestations]
if len(a) == 0 {
return a
}

var limit uint64
if a[0].Version() == version.Phase0 {
limit = params.BeaconConfig().MaxAttestations
} else {
limit = params.BeaconConfig().MaxAttestationsElectra
}
if uint64(len(a)) > limit {
return a[:limit]
Comment on lines +192 to +203
Copy link
Member

Choose a reason for hiding this comment

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

Should this be tested?

}
return a
}
Expand All @@ -182,13 +212,13 @@ func (a proposerAtts) dedup() (proposerAtts, error) {
if len(a) < 2 {
return a, nil
}
attsByDataRoot := make(map[[32]byte][]ethpb.Att, len(a))
attsByDataRoot := make(map[attestation.Id][]ethpb.Att, len(a))
for _, att := range a {
attDataRoot, err := att.GetData().HashTreeRoot()
id, err := attestation.NewId(att, attestation.Data)
if err != nil {
continue
return nil, errors.Wrap(err, "failed to create attestation ID")
Copy link
Contributor

Choose a reason for hiding this comment

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

Keeping errors the same

Suggested change
return nil, errors.Wrap(err, "failed to create attestation ID")
return nil, errors.Wrap(err, "could not create attestation ID")

}
attsByDataRoot[attDataRoot] = append(attsByDataRoot[attDataRoot], att)
attsByDataRoot[id] = append(attsByDataRoot[id], att)
}

uniqAtts := make([]ethpb.Att, 0, len(a))
Expand Down
Loading
Loading