Skip to content

Commit 153d187

Browse files
rkapkarkapka
authored
Separate type for unaggregated network attestations (#14659)
* definitions and gossip * validator * broadcast * broadcast the correct att depending on version * small updates * don't check bits after Electra * nitpick * tests * changelog <3 * review * more review * review yet again * try a different design * fix gossip issues * cleanup * tests * reduce cognitive complexity * Preston's review * move changelog entry to unreleased section * fix pending atts pool issues * reviews * Potuz's comments * test fixes
1 parent 80aa811 commit 153d187

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

50 files changed

+1793
-930
lines changed

‎CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ The format is based on Keep a Changelog, and this project adheres to Semantic Ve
2121
- DB optimization for saving light client bootstraps (save unique sync committees only).
2222
- Trace IDONTWANT Messages in Pubsub.
2323
- Add Fulu fork boilerplate.
24+
- Separate type for unaggregated network attestations. [PR](https://github.com/prysmaticlabs/prysm/pull/14659)
2425

2526
### Changed
2627

‎api/server/structs/conversions.go

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -432,6 +432,32 @@ func (a *AttestationElectra) ToConsensus() (*eth.AttestationElectra, error) {
432432
}, nil
433433
}
434434

435+
func (a *SingleAttestation) ToConsensus() (*eth.SingleAttestation, error) {
436+
ci, err := strconv.ParseUint(a.CommitteeIndex, 10, 64)
437+
if err != nil {
438+
return nil, server.NewDecodeError(err, "CommitteeIndex")
439+
}
440+
ai, err := strconv.ParseUint(a.AttesterIndex, 10, 64)
441+
if err != nil {
442+
return nil, server.NewDecodeError(err, "AttesterIndex")
443+
}
444+
data, err := a.Data.ToConsensus()
445+
if err != nil {
446+
return nil, server.NewDecodeError(err, "Data")
447+
}
448+
sig, err := bytesutil.DecodeHexWithLength(a.Signature, fieldparams.BLSSignatureLength)
449+
if err != nil {
450+
return nil, server.NewDecodeError(err, "Signature")
451+
}
452+
453+
return &eth.SingleAttestation{
454+
CommitteeId: primitives.CommitteeIndex(ci),
455+
AttesterIndex: primitives.ValidatorIndex(ai),
456+
Data: data,
457+
Signature: sig,
458+
}, nil
459+
}
460+
435461
func AttElectraFromConsensus(a *eth.AttestationElectra) *AttestationElectra {
436462
return &AttestationElectra{
437463
AggregationBits: hexutil.Encode(a.AggregationBits),

‎api/server/structs/other.go

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,13 @@ type AttestationElectra struct {
3636
CommitteeBits string `json:"committee_bits"`
3737
}
3838

39+
type SingleAttestation struct {
40+
CommitteeIndex string `json:"committee_index"`
41+
AttesterIndex string `json:"attester_index"`
42+
Data *AttestationData `json:"data"`
43+
Signature string `json:"signature"`
44+
}
45+
3946
type AttestationData struct {
4047
Slot string `json:"slot"`
4148
CommitteeIndex string `json:"index"`

‎beacon-chain/core/helpers/attestation.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,9 @@ func ValidateNilAttestation(attestation ethpb.Att) error {
3232
if attestation.GetData().Target == nil {
3333
return errors.New("attestation's target can't be nil")
3434
}
35+
if attestation.IsSingle() {
36+
return nil
37+
}
3538
if attestation.GetAggregationBits() == nil {
3639
return errors.New("attestation's bitfield can't be nil")
3740
}

‎beacon-chain/core/helpers/attestation_test.go

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -308,6 +308,16 @@ func TestValidateNilAttestation(t *testing.T) {
308308
},
309309
errString: "",
310310
},
311+
{
312+
name: "single attestation",
313+
attestation: &ethpb.SingleAttestation{
314+
Data: &ethpb.AttestationData{
315+
Target: &ethpb.Checkpoint{},
316+
Source: &ethpb.Checkpoint{},
317+
},
318+
},
319+
errString: "",
320+
},
311321
}
312322
for _, tt := range tests {
313323
t.Run(tt.name, func(t *testing.T) {

‎beacon-chain/core/helpers/beacon_committee_test.go

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,13 @@ func TestVerifyBitfieldLength_OK(t *testing.T) {
9797
assert.NoError(t, helpers.VerifyBitfieldLength(bf, committeeSize), "Bitfield is not validated when it was supposed to be")
9898
}
9999

100+
func TestVerifyBitfieldLength_Incorrect(t *testing.T) {
101+
helpers.ClearCache()
102+
103+
bf := bitfield.NewBitlist(1)
104+
require.ErrorContains(t, "wanted participants bitfield length 2, got: 1", helpers.VerifyBitfieldLength(bf, 2))
105+
}
106+
100107
func TestCommitteeAssignments_CannotRetrieveFutureEpoch(t *testing.T) {
101108
helpers.ClearCache()
102109

‎beacon-chain/p2p/gossip_topic_mappings.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ func GossipTopicMappings(topic string, epoch primitives.Epoch) proto.Message {
5050
return gossipMessage(topic)
5151
case AttestationSubnetTopicFormat:
5252
if epoch >= params.BeaconConfig().ElectraForkEpoch {
53-
return &ethpb.AttestationElectra{}
53+
return &ethpb.SingleAttestation{}
5454
}
5555
return gossipMessage(topic)
5656
case AttesterSlashingSubnetTopicFormat:
@@ -109,7 +109,7 @@ func init() {
109109

110110
// Specially handle Electra objects.
111111
GossipTypeMapping[reflect.TypeOf(&ethpb.SignedBeaconBlockElectra{})] = BlockSubnetTopicFormat
112-
GossipTypeMapping[reflect.TypeOf(&ethpb.AttestationElectra{})] = AttestationSubnetTopicFormat
112+
GossipTypeMapping[reflect.TypeOf(&ethpb.SingleAttestation{})] = AttestationSubnetTopicFormat
113113
GossipTypeMapping[reflect.TypeOf(&ethpb.AttesterSlashingElectra{})] = AttesterSlashingSubnetTopicFormat
114114
GossipTypeMapping[reflect.TypeOf(&ethpb.SignedAggregateAttestationAndProofElectra{})] = AggregateAndProofSubnetTopicFormat
115115

‎beacon-chain/p2p/gossip_topic_mappings_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -121,7 +121,7 @@ func TestGossipTopicMappings_CorrectType(t *testing.T) {
121121
_, ok = pMessage.(*ethpb.SignedBeaconBlockElectra)
122122
assert.Equal(t, true, ok)
123123
pMessage = GossipTopicMappings(AttestationSubnetTopicFormat, electraForkEpoch)
124-
_, ok = pMessage.(*ethpb.AttestationElectra)
124+
_, ok = pMessage.(*ethpb.SingleAttestation)
125125
assert.Equal(t, true, ok)
126126
pMessage = GossipTopicMappings(AttesterSlashingSubnetTopicFormat, electraForkEpoch)
127127
_, ok = pMessage.(*ethpb.AttesterSlashingElectra)

‎beacon-chain/p2p/types/object_mapping.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,7 @@ func InitializeDataMaps() {
120120
return &ethpb.Attestation{}, nil
121121
},
122122
bytesutil.ToBytes4(params.BeaconConfig().ElectraForkVersion): func() (ethpb.Att, error) {
123-
return &ethpb.AttestationElectra{}, nil
123+
return &ethpb.SingleAttestation{}, nil
124124
},
125125
bytesutil.ToBytes4(params.BeaconConfig().FuluForkVersion): func() (ethpb.Att, error) {
126126
return &ethpb.AttestationElectra{}, nil

‎beacon-chain/rpc/endpoints.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -532,6 +532,7 @@ func (s *Service) beaconEndpoints(
532532
FinalizationFetcher: s.cfg.FinalizationFetcher,
533533
ForkchoiceFetcher: s.cfg.ForkchoiceFetcher,
534534
CoreService: coreService,
535+
AttestationStateFetcher: s.cfg.AttestationReceiver,
535536
}
536537

537538
const namespace = "beacon"

‎beacon-chain/rpc/eth/beacon/handlers_pool.go

Lines changed: 25 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -285,8 +285,11 @@ func (s *Server) SubmitAttestationsV2(w http.ResponseWriter, r *http.Request) {
285285
}
286286
}
287287

288-
func (s *Server) handleAttestationsElectra(ctx context.Context, data json.RawMessage) (attFailures []*server.IndexedVerificationFailure, failedBroadcasts []string, err error) {
289-
var sourceAttestations []*structs.AttestationElectra
288+
func (s *Server) handleAttestationsElectra(
289+
ctx context.Context,
290+
data json.RawMessage,
291+
) (attFailures []*server.IndexedVerificationFailure, failedBroadcasts []string, err error) {
292+
var sourceAttestations []*structs.SingleAttestation
290293

291294
if err = json.Unmarshal(data, &sourceAttestations); err != nil {
292295
return nil, nil, errors.Wrap(err, "failed to unmarshal attestation")
@@ -296,7 +299,7 @@ func (s *Server) handleAttestationsElectra(ctx context.Context, data json.RawMes
296299
return nil, nil, errors.New("no data submitted")
297300
}
298301

299-
var validAttestations []*eth.AttestationElectra
302+
var validAttestations []*eth.SingleAttestation
300303
for i, sourceAtt := range sourceAttestations {
301304
att, err := sourceAtt.ToConsensus()
302305
if err != nil {
@@ -316,31 +319,32 @@ func (s *Server) handleAttestationsElectra(ctx context.Context, data json.RawMes
316319
validAttestations = append(validAttestations, att)
317320
}
318321

319-
for i, att := range validAttestations {
320-
// Broadcast the unaggregated attestation on a feed to notify other services in the beacon node
321-
// of a received unaggregated attestation.
322-
// Note we can't send for aggregated att because we don't have selection proof.
323-
if !att.IsAggregated() {
324-
s.OperationNotifier.OperationFeed().Send(&feed.Event{
325-
Type: operation.UnaggregatedAttReceived,
326-
Data: &operation.UnAggregatedAttReceivedData{
327-
Attestation: att,
328-
},
329-
})
322+
for i, singleAtt := range validAttestations {
323+
targetState, err := s.AttestationStateFetcher.AttestationTargetState(ctx, singleAtt.Data.Target)
324+
if err != nil {
325+
return nil, nil, errors.Wrap(err, "could not get target state for attestation")
326+
}
327+
committee, err := corehelpers.BeaconCommitteeFromState(ctx, targetState, singleAtt.Data.Slot, singleAtt.CommitteeId)
328+
if err != nil {
329+
return nil, nil, errors.Wrap(err, "could not get committee for attestation")
330330
}
331+
att := singleAtt.ToAttestationElectra(committee)
332+
333+
s.OperationNotifier.OperationFeed().Send(&feed.Event{
334+
Type: operation.UnaggregatedAttReceived,
335+
Data: &operation.UnAggregatedAttReceivedData{
336+
Attestation: att,
337+
},
338+
})
331339

332340
wantedEpoch := slots.ToEpoch(att.Data.Slot)
333341
vals, err := s.HeadFetcher.HeadValidatorsIndices(ctx, wantedEpoch)
334342
if err != nil {
335343
failedBroadcasts = append(failedBroadcasts, strconv.Itoa(i))
336344
continue
337345
}
338-
committeeIndex, err := att.GetCommitteeIndex()
339-
if err != nil {
340-
return nil, nil, errors.Wrap(err, "failed to retrieve attestation committee index")
341-
}
342-
subnet := corehelpers.ComputeSubnetFromCommitteeAndSlot(uint64(len(vals)), committeeIndex, att.Data.Slot)
343-
if err = s.Broadcaster.BroadcastAttestation(ctx, subnet, att); err != nil {
346+
subnet := corehelpers.ComputeSubnetFromCommitteeAndSlot(uint64(len(vals)), att.GetCommitteeIndex(), att.Data.Slot)
347+
if err = s.Broadcaster.BroadcastAttestation(ctx, subnet, singleAtt); err != nil {
344348
log.WithError(err).Errorf("could not broadcast attestation at index %d", i)
345349
failedBroadcasts = append(failedBroadcasts, strconv.Itoa(i))
346350
continue
@@ -350,13 +354,9 @@ func (s *Server) handleAttestationsElectra(ctx context.Context, data json.RawMes
350354
if err = s.AttestationCache.Add(att); err != nil {
351355
log.WithError(err).Error("could not save attestation")
352356
}
353-
} else if att.IsAggregated() {
354-
if err = s.AttestationsPool.SaveAggregatedAttestation(att); err != nil {
355-
log.WithError(err).Error("could not save aggregated attestation")
356-
}
357357
} else {
358358
if err = s.AttestationsPool.SaveUnaggregatedAttestation(att); err != nil {
359-
log.WithError(err).Error("could not save unaggregated attestation")
359+
log.WithError(err).Error("could not save attestation")
360360
}
361361
}
362362
}

‎beacon-chain/rpc/eth/beacon/handlers_pool_test.go

Lines changed: 18 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -498,13 +498,17 @@ func TestSubmitAttestations(t *testing.T) {
498498
c.SlotsPerEpoch = 1
499499
params.OverrideBeaconConfig(c)
500500

501-
_, keys, err := util.DeterministicDepositsAndKeys(1)
501+
_, keys, err := util.DeterministicDepositsAndKeys(2)
502502
require.NoError(t, err)
503503
validators := []*ethpbv1alpha1.Validator{
504504
{
505505
PublicKey: keys[0].PublicKey().Marshal(),
506506
ExitEpoch: params.BeaconConfig().FarFutureEpoch,
507507
},
508+
{
509+
PublicKey: keys[1].PublicKey().Marshal(),
510+
ExitEpoch: params.BeaconConfig().FarFutureEpoch,
511+
},
508512
}
509513
bs, err := util.NewBeaconState(func(state *ethpbv1alpha1.BeaconState) error {
510514
state.Validators = validators
@@ -521,9 +525,10 @@ func TestSubmitAttestations(t *testing.T) {
521525

522526
chainService := &blockchainmock.ChainService{State: bs}
523527
s := &Server{
524-
HeadFetcher: chainService,
525-
ChainInfoFetcher: chainService,
526-
OperationNotifier: &blockchainmock.MockOperationNotifier{},
528+
HeadFetcher: chainService,
529+
ChainInfoFetcher: chainService,
530+
OperationNotifier: &blockchainmock.MockOperationNotifier{},
531+
AttestationStateFetcher: chainService,
527532
}
528533
t.Run("V1", func(t *testing.T) {
529534
t.Run("single", func(t *testing.T) {
@@ -732,7 +737,7 @@ func TestSubmitAttestations(t *testing.T) {
732737
assert.Equal(t, http.StatusOK, writer.Code)
733738
assert.Equal(t, true, broadcaster.BroadcastCalled.Load())
734739
assert.Equal(t, 1, broadcaster.NumAttestations())
735-
assert.Equal(t, "0x03", hexutil.Encode(broadcaster.BroadcastAttestations[0].GetAggregationBits()))
740+
assert.Equal(t, primitives.ValidatorIndex(1), broadcaster.BroadcastAttestations[0].GetAttestingIndex())
736741
assert.Equal(t, "0x8146f4397bfd8fd057ebbcd6a67327bdc7ed5fb650533edcb6377b650dea0b6da64c14ecd60846d5c0a0cd43893d6972092500f82c9d8a955e2b58c5ed3cbe885d84008ace6bd86ba9e23652f58e2ec207cec494c916063257abf285b9b15b15", hexutil.Encode(broadcaster.BroadcastAttestations[0].GetSignature()))
737742
assert.Equal(t, primitives.Slot(0), broadcaster.BroadcastAttestations[0].GetData().Slot)
738743
assert.Equal(t, primitives.CommitteeIndex(0), broadcaster.BroadcastAttestations[0].GetData().CommitteeIndex)
@@ -2344,8 +2349,8 @@ var (
23442349
]`
23452350
singleAttElectra = `[
23462351
{
2347-
"aggregation_bits": "0x03",
2348-
"committee_bits": "0x0100000000000000",
2352+
"committee_index": "0",
2353+
"attester_index": "1",
23492354
"signature": "0x8146f4397bfd8fd057ebbcd6a67327bdc7ed5fb650533edcb6377b650dea0b6da64c14ecd60846d5c0a0cd43893d6972092500f82c9d8a955e2b58c5ed3cbe885d84008ace6bd86ba9e23652f58e2ec207cec494c916063257abf285b9b15b15",
23502355
"data": {
23512356
"slot": "0",
@@ -2364,8 +2369,8 @@ var (
23642369
]`
23652370
multipleAttsElectra = `[
23662371
{
2367-
"aggregation_bits": "0x03",
2368-
"committee_bits": "0x0100000000000000",
2372+
"committee_index": "0",
2373+
"attester_index": "0",
23692374
"signature": "0x8146f4397bfd8fd057ebbcd6a67327bdc7ed5fb650533edcb6377b650dea0b6da64c14ecd60846d5c0a0cd43893d6972092500f82c9d8a955e2b58c5ed3cbe885d84008ace6bd86ba9e23652f58e2ec207cec494c916063257abf285b9b15b15",
23702375
"data": {
23712376
"slot": "0",
@@ -2382,8 +2387,8 @@ var (
23822387
}
23832388
},
23842389
{
2385-
"aggregation_bits": "0x03",
2386-
"committee_bits": "0x0100000000000000",
2390+
"committee_index": "0",
2391+
"attester_index": "1",
23872392
"signature": "0x8146f4397bfd8fd057ebbcd6a67327bdc7ed5fb650533edcb6377b650dea0b6da64c14ecd60846d5c0a0cd43893d6972092500f82c9d8a955e2b58c5ed3cbe885d84008ace6bd86ba9e23652f58e2ec207cec494c916063257abf285b9b15b15",
23882393
"data": {
23892394
"slot": "0",
@@ -2403,8 +2408,8 @@ var (
24032408
// signature is invalid
24042409
invalidAttElectra = `[
24052410
{
2406-
"aggregation_bits": "0x03",
2407-
"committee_bits": "0x0100000000000000",
2411+
"committee_index": "0",
2412+
"attester_index": "0",
24082413
"signature": "0x000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000",
24092414
"data": {
24102415
"slot": "0",

‎beacon-chain/rpc/eth/beacon/server.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,4 +50,5 @@ type Server struct {
5050
BLSChangesPool blstoexec.PoolManager
5151
ForkchoiceFetcher blockchain.ForkchoiceFetcher
5252
CoreService *core.Service
53+
AttestationStateFetcher blockchain.AttestationStateFetcher
5354
}

‎beacon-chain/rpc/eth/validator/handlers.go

Lines changed: 9 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -198,35 +198,20 @@ func matchingAtts(atts []ethpbalpha.Att, slot primitives.Slot, attDataRoot []byt
198198
continue
199199
}
200200

201+
root, err := att.GetData().HashTreeRoot()
202+
if err != nil {
203+
return nil, errors.Wrap(err, "could not get attestation data root")
204+
}
205+
if !bytes.Equal(root[:], attDataRoot) {
206+
continue
207+
}
208+
201209
// We ignore the committee index from the request before Electra.
202210
// This is because before Electra the committee index is part of the attestation data,
203211
// meaning that comparing the data root is sufficient.
204212
// Post-Electra the committee index in the data root is always 0, so we need to
205213
// compare the committee index separately.
206-
if postElectra {
207-
if att.Version() >= version.Electra {
208-
ci, err := att.GetCommitteeIndex()
209-
if err != nil {
210-
return nil, err
211-
}
212-
if ci != index {
213-
continue
214-
}
215-
} else {
216-
continue
217-
}
218-
} else {
219-
// If postElectra is false and att.Version >= version.Electra, ignore the attestation.
220-
if att.Version() >= version.Electra {
221-
continue
222-
}
223-
}
224-
225-
root, err := att.GetData().HashTreeRoot()
226-
if err != nil {
227-
return nil, errors.Wrap(err, "could not get attestation data root")
228-
}
229-
if bytes.Equal(root[:], attDataRoot) {
214+
if (!postElectra && att.Version() < version.Electra) || (postElectra && att.Version() >= version.Electra && att.GetCommitteeIndex() == index) {
230215
result = append(result, att)
231216
}
232217
}

0 commit comments

Comments
 (0)