From 2b2ef4f37cd36cb1389a6f1df58dadfd765f4ae1 Mon Sep 17 00:00:00 2001 From: terence tsao Date: Sat, 21 Sep 2019 09:43:18 -0700 Subject: [PATCH] Added AggregatedAttestation helper (#3539) --- .../blockchain/forkchoice/BUILD.bazel | 1 - .../forkchoice/process_attestation.go | 20 +++---------- beacon-chain/blockchain/forkchoice/service.go | 2 -- beacon-chain/blockchain/receive_block.go | 2 +- beacon-chain/core/helpers/attestation.go | 30 +++++++++++++++++++ beacon-chain/core/helpers/attestation_test.go | 29 ++++++++++++++++++ beacon-chain/operations/BUILD.bazel | 2 +- beacon-chain/operations/service.go | 28 +++++------------ beacon-chain/sync/service.go | 14 ++++----- 9 files changed, 79 insertions(+), 49 deletions(-) diff --git a/beacon-chain/blockchain/forkchoice/BUILD.bazel b/beacon-chain/blockchain/forkchoice/BUILD.bazel index aacae67a8..eccddf7dd 100644 --- a/beacon-chain/blockchain/forkchoice/BUILD.bazel +++ b/beacon-chain/blockchain/forkchoice/BUILD.bazel @@ -21,7 +21,6 @@ go_library( "//beacon-chain/db/filters:go_default_library", "//proto/beacon/p2p/v1:go_default_library", "//proto/eth/v1alpha1:go_default_library", - "//shared/bls:go_default_library", "//shared/bytesutil:go_default_library", "//shared/hashutil:go_default_library", "//shared/params:go_default_library", diff --git a/beacon-chain/blockchain/forkchoice/process_attestation.go b/beacon-chain/blockchain/forkchoice/process_attestation.go index 7d14a77dc..34531160a 100644 --- a/beacon-chain/blockchain/forkchoice/process_attestation.go +++ b/beacon-chain/blockchain/forkchoice/process_attestation.go @@ -14,7 +14,6 @@ import ( "github.com/prysmaticlabs/prysm/beacon-chain/core/state" pb "github.com/prysmaticlabs/prysm/proto/beacon/p2p/v1" ethpb "github.com/prysmaticlabs/prysm/proto/eth/v1alpha1" - "github.com/prysmaticlabs/prysm/shared/bls" "github.com/prysmaticlabs/prysm/shared/bytesutil" "github.com/prysmaticlabs/prysm/shared/hashutil" "github.com/prysmaticlabs/prysm/shared/params" @@ -203,23 +202,12 @@ func (s *Store) aggregateAttestation(ctx context.Context, att *ethpb.Attestation return err } - incomingAttBits := att.AggregationBits if a, ok := s.attsQueue[root]; ok { - if !a.AggregationBits.Contains(incomingAttBits) { - newBits := a.AggregationBits.Or(incomingAttBits) - incomingSig, err := bls.SignatureFromBytes(att.Signature) - if err != nil { - return err - } - currentSig, err := bls.SignatureFromBytes(a.Signature) - if err != nil { - return err - } - aggregatedSig := bls.AggregateSignatures([]*bls.Signature{currentSig, incomingSig}) - a.Signature = aggregatedSig.Marshal() - a.AggregationBits = newBits - s.attsQueue[root] = a + a, err := helpers.AggregateAttestation(a, att) + if err != nil { + return nil } + s.attsQueue[root] = a return nil } diff --git a/beacon-chain/blockchain/forkchoice/service.go b/beacon-chain/blockchain/forkchoice/service.go index 23e1c5d93..f0276221c 100644 --- a/beacon-chain/blockchain/forkchoice/service.go +++ b/beacon-chain/blockchain/forkchoice/service.go @@ -252,5 +252,3 @@ func (s *Store) JustifiedCheckpt() *ethpb.Checkpoint { func (s *Store) FinalizedCheckpt() *ethpb.Checkpoint { return proto.Clone(s.finalizedCheckpt).(*ethpb.Checkpoint) } - - diff --git a/beacon-chain/blockchain/receive_block.go b/beacon-chain/blockchain/receive_block.go index c6de24dfc..97a6eb004 100644 --- a/beacon-chain/blockchain/receive_block.go +++ b/beacon-chain/blockchain/receive_block.go @@ -41,7 +41,7 @@ func (s *Service) ReceiveBlock(ctx context.Context, block *ethpb.BeaconBlock) er } log.WithFields(logrus.Fields{ "blockRoot": hex.EncodeToString(root[:]), - }).Info("Broadcasting block") + }).Debug("Broadcasting block") if err := s.ReceiveBlockNoPubsub(ctx, block); err != nil { return err diff --git a/beacon-chain/core/helpers/attestation.go b/beacon-chain/core/helpers/attestation.go index af21291bb..deea13566 100644 --- a/beacon-chain/core/helpers/attestation.go +++ b/beacon-chain/core/helpers/attestation.go @@ -4,6 +4,7 @@ import ( "github.com/pkg/errors" pb "github.com/prysmaticlabs/prysm/proto/beacon/p2p/v1" ethpb "github.com/prysmaticlabs/prysm/proto/eth/v1alpha1" + "github.com/prysmaticlabs/prysm/shared/bls" "github.com/prysmaticlabs/prysm/shared/params" ) @@ -49,3 +50,32 @@ func AttestationDataSlot(state *pb.BeaconState, data *ethpb.AttestationData) (ui return StartSlot(data.Target.Epoch) + (offset / (committeeCount / params.BeaconConfig().SlotsPerEpoch)), nil } + +// AggregateAttestation aggregates attestations a1 and a2 together. +func AggregateAttestation(a1 *ethpb.Attestation, a2 *ethpb.Attestation) (*ethpb.Attestation, error) { + baseAtt := a1 + newAtt := a2 + if a2.AggregationBits.Count() > a1.AggregationBits.Count() { + baseAtt = a2 + newAtt = a1 + } + if baseAtt.AggregationBits.Contains(newAtt.AggregationBits) { + return baseAtt, nil + } + + newBits := baseAtt.AggregationBits.Or(newAtt.AggregationBits) + newSig, err := bls.SignatureFromBytes(newAtt.Signature) + if err != nil { + return nil, err + } + baseSig, err := bls.SignatureFromBytes(baseAtt.Signature) + if err != nil { + return nil, err + } + + aggregatedSig := bls.AggregateSignatures([]*bls.Signature{baseSig, newSig}) + baseAtt.Signature = aggregatedSig.Marshal() + baseAtt.AggregationBits = newBits + + return baseAtt, nil +} diff --git a/beacon-chain/core/helpers/attestation_test.go b/beacon-chain/core/helpers/attestation_test.go index d1f3ddf87..228f855e4 100644 --- a/beacon-chain/core/helpers/attestation_test.go +++ b/beacon-chain/core/helpers/attestation_test.go @@ -1,8 +1,10 @@ package helpers_test import ( + "reflect" "testing" + "github.com/prysmaticlabs/go-bitfield" "github.com/prysmaticlabs/prysm/beacon-chain/core/helpers" "github.com/prysmaticlabs/prysm/beacon-chain/core/state" pb "github.com/prysmaticlabs/prysm/proto/beacon/p2p/v1" @@ -84,3 +86,30 @@ func TestAttestationDataSlot_ReturnsErrorWhenTargetEpochLessThanCurrentEpoch(t * t.Logf("attestation slot=%v", s) } } + +func TestAggregateAttestation(t *testing.T) { + tests := []struct { + a1 *ethpb.Attestation + a2 *ethpb.Attestation + want *ethpb.Attestation + }{ + {a1: ðpb.Attestation{AggregationBits: []byte{}}, + a2: ðpb.Attestation{AggregationBits: []byte{}}, + want: ðpb.Attestation{AggregationBits: []byte{}}}, + {a1: ðpb.Attestation{AggregationBits: bitfield.Bitlist{0x02}}, + a2: ðpb.Attestation{AggregationBits: bitfield.Bitlist{0x03}}, + want: ðpb.Attestation{AggregationBits: []byte{0x03}}}, + {a1: ðpb.Attestation{AggregationBits: bitfield.Bitlist{0x03}}, + a2: ðpb.Attestation{AggregationBits: bitfield.Bitlist{0x02}}, + want: ðpb.Attestation{AggregationBits: []byte{0x03}}}, + } + for _, tt := range tests { + got, err := helpers.AggregateAttestation(tt.a1, tt.a2) + if err != nil { + t.Fatal(err) + } + if !reflect.DeepEqual(got, tt.want) { + t.Errorf("AggregateAttestation() = %v, want %v", got, tt.want) + } + } +} diff --git a/beacon-chain/operations/BUILD.bazel b/beacon-chain/operations/BUILD.bazel index 6e2d8a270..5ef7e6a81 100644 --- a/beacon-chain/operations/BUILD.bazel +++ b/beacon-chain/operations/BUILD.bazel @@ -7,10 +7,10 @@ go_library( visibility = ["//beacon-chain:__subpackages__"], deps = [ "//beacon-chain/core/blocks:go_default_library", + "//beacon-chain/core/helpers:go_default_library", "//beacon-chain/core/state:go_default_library", "//beacon-chain/db:go_default_library", "//proto/eth/v1alpha1:go_default_library", - "//shared/bls:go_default_library", "//shared/event:go_default_library", "//shared/hashutil:go_default_library", "//shared/messagehandler:go_default_library", diff --git a/beacon-chain/operations/service.go b/beacon-chain/operations/service.go index dd7fea843..3b756ff7c 100644 --- a/beacon-chain/operations/service.go +++ b/beacon-chain/operations/service.go @@ -13,10 +13,10 @@ import ( "github.com/pkg/errors" "github.com/prysmaticlabs/go-ssz" "github.com/prysmaticlabs/prysm/beacon-chain/core/blocks" + "github.com/prysmaticlabs/prysm/beacon-chain/core/helpers" "github.com/prysmaticlabs/prysm/beacon-chain/core/state" "github.com/prysmaticlabs/prysm/beacon-chain/db" ethpb "github.com/prysmaticlabs/prysm/proto/eth/v1alpha1" - "github.com/prysmaticlabs/prysm/shared/bls" "github.com/prysmaticlabs/prysm/shared/event" "github.com/prysmaticlabs/prysm/shared/hashutil" handler "github.com/prysmaticlabs/prysm/shared/messagehandler" @@ -214,31 +214,17 @@ func (s *Service) HandleAttestation(ctx context.Context, message proto.Message) } } - incomingAttBits := attestation.AggregationBits if s.beaconDB.HasAttestation(ctx, root) { dbAtt, err := s.beaconDB.Attestation(ctx, root) if err != nil { return err } - - if !dbAtt.AggregationBits.Contains(incomingAttBits) { - newAggregationBits := dbAtt.AggregationBits.Or(incomingAttBits) - incomingAttSig, err := bls.SignatureFromBytes(attestation.Signature) - if err != nil { - return err - } - dbSig, err := bls.SignatureFromBytes(dbAtt.Signature) - if err != nil { - return err - } - aggregatedSig := bls.AggregateSignatures([]*bls.Signature{dbSig, incomingAttSig}) - dbAtt.Signature = aggregatedSig.Marshal() - dbAtt.AggregationBits = newAggregationBits - if err := s.beaconDB.SaveAttestation(ctx, dbAtt); err != nil { - return err - } - } else { - return nil + attestation, err = helpers.AggregateAttestation(dbAtt, attestation) + if err != nil { + return err + } + if err := s.beaconDB.SaveAttestation(ctx, attestation); err != nil { + return err } } else { if err := s.beaconDB.SaveAttestation(ctx, attestation); err != nil { diff --git a/beacon-chain/sync/service.go b/beacon-chain/sync/service.go index a7681e710..a56d7cfc2 100644 --- a/beacon-chain/sync/service.go +++ b/beacon-chain/sync/service.go @@ -37,12 +37,12 @@ type blockchainService interface { // NewRegularSync service. func NewRegularSync(cfg *Config) *RegularSync { r := &RegularSync{ - ctx: context.Background(), - db: cfg.DB, - p2p: cfg.P2P, - operations: cfg.Operations, - chain: cfg.Chain, - initialSync: cfg.InitialSync, + ctx: context.Background(), + db: cfg.DB, + p2p: cfg.P2P, + operations: cfg.Operations, + chain: cfg.Chain, + initialSync: cfg.InitialSync, slotToPendingBlocks: make(map[uint64]*ethpb.BeaconBlock), seenPendingBlocks: make(map[[32]byte]bool), } @@ -65,7 +65,7 @@ type RegularSync struct { seenPendingBlocks map[[32]byte]bool pendingQueueLock sync.RWMutex chainStarted bool - initialSync Checker + initialSync Checker } // Start the regular sync service.