From ecce4288964056502d3cff7368d542129791830f Mon Sep 17 00:00:00 2001 From: terence tsao Date: Mon, 1 Jun 2020 13:41:03 -0700 Subject: [PATCH 1/4] Use state util to get block root --- beacon-chain/blockchain/BUILD.bazel | 1 - beacon-chain/blockchain/process_block_helpers.go | 3 +-- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/beacon-chain/blockchain/BUILD.bazel b/beacon-chain/blockchain/BUILD.bazel index 411b374d01c2..12035a65b413 100644 --- a/beacon-chain/blockchain/BUILD.bazel +++ b/beacon-chain/blockchain/BUILD.bazel @@ -55,7 +55,6 @@ go_library( "@com_github_prometheus_client_golang//prometheus:go_default_library", "@com_github_prometheus_client_golang//prometheus/promauto:go_default_library", "@com_github_prysmaticlabs_ethereumapis//eth/v1alpha1:go_default_library", - "@com_github_prysmaticlabs_go_ssz//:go_default_library", "@com_github_sirupsen_logrus//:go_default_library", "@io_opencensus_go//trace:go_default_library", ], diff --git a/beacon-chain/blockchain/process_block_helpers.go b/beacon-chain/blockchain/process_block_helpers.go index 6f1c172e6185..0a0175c632a6 100644 --- a/beacon-chain/blockchain/process_block_helpers.go +++ b/beacon-chain/blockchain/process_block_helpers.go @@ -7,7 +7,6 @@ import ( "github.com/pkg/errors" ethpb "github.com/prysmaticlabs/ethereumapis/eth/v1alpha1" - "github.com/prysmaticlabs/go-ssz" "github.com/prysmaticlabs/prysm/beacon-chain/core/helpers" "github.com/prysmaticlabs/prysm/beacon-chain/db/filters" stateTrie "github.com/prysmaticlabs/prysm/beacon-chain/state" @@ -480,7 +479,7 @@ func (s *Service) fillInForkChoiceMissingBlocks(ctx context.Context, blk *ethpb. // Lower slots should be at the end of the list. for i := len(pendingNodes) - 1; i >= 0; i-- { b := pendingNodes[i] - r, err := ssz.HashTreeRoot(b) + r, err := stateutil.BlockRoot(b) if err != nil { return err } From df89cb1d383b982365eb680a16327979092972fb Mon Sep 17 00:00:00 2001 From: terence tsao Date: Wed, 3 Jun 2020 11:40:32 -0700 Subject: [PATCH 2/4] Also save unaggregated att --- beacon-chain/sync/subscriber_beacon_aggregate_proof.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/beacon-chain/sync/subscriber_beacon_aggregate_proof.go b/beacon-chain/sync/subscriber_beacon_aggregate_proof.go index 5ed77b0b188a..6a101f101dbc 100644 --- a/beacon-chain/sync/subscriber_beacon_aggregate_proof.go +++ b/beacon-chain/sync/subscriber_beacon_aggregate_proof.go @@ -9,6 +9,7 @@ import ( ethpb "github.com/prysmaticlabs/ethereumapis/eth/v1alpha1" "github.com/prysmaticlabs/prysm/beacon-chain/core/feed" "github.com/prysmaticlabs/prysm/beacon-chain/core/feed/operation" + "github.com/prysmaticlabs/prysm/beacon-chain/core/helpers" ) // beaconAggregateProofSubscriber forwards the incoming validated aggregated attestation and proof to the @@ -33,5 +34,10 @@ func (r *Service) beaconAggregateProofSubscriber(ctx context.Context, msg proto. }, }) + // An unaggregated attestation can make it here. It’s valid, the aggregator it just itself, although it means poor performance for the subnet. + if !helpers.IsAggregated(a.Message.Aggregate) { + r.attPool.SaveUnaggregatedAttestation(a.Message.Aggregate) + } + return r.attPool.SaveAggregatedAttestation(a.Message.Aggregate) } From 90cfc793270df54c102b9495fb74ab89086b0bd3 Mon Sep 17 00:00:00 2001 From: terence tsao Date: Wed, 3 Jun 2020 11:44:33 -0700 Subject: [PATCH 3/4] Handle error --- beacon-chain/sync/subscriber_beacon_aggregate_proof.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/beacon-chain/sync/subscriber_beacon_aggregate_proof.go b/beacon-chain/sync/subscriber_beacon_aggregate_proof.go index 6a101f101dbc..7e6609325983 100644 --- a/beacon-chain/sync/subscriber_beacon_aggregate_proof.go +++ b/beacon-chain/sync/subscriber_beacon_aggregate_proof.go @@ -36,7 +36,9 @@ func (r *Service) beaconAggregateProofSubscriber(ctx context.Context, msg proto. // An unaggregated attestation can make it here. It’s valid, the aggregator it just itself, although it means poor performance for the subnet. if !helpers.IsAggregated(a.Message.Aggregate) { - r.attPool.SaveUnaggregatedAttestation(a.Message.Aggregate) + if err := r.attPool.SaveUnaggregatedAttestation(a.Message.Aggregate); err != nil { + return err + } } return r.attPool.SaveAggregatedAttestation(a.Message.Aggregate) From dd1895f903680f3df380a491b1910675ffafe4a1 Mon Sep 17 00:00:00 2001 From: terence tsao Date: Wed, 3 Jun 2020 12:01:23 -0700 Subject: [PATCH 4/4] Test --- .../sync/subscriber_beacon_aggregate_proof.go | 4 +--- .../subscriber_beacon_aggregate_proof_test.go | 23 ++++++++++++++++++- 2 files changed, 23 insertions(+), 4 deletions(-) diff --git a/beacon-chain/sync/subscriber_beacon_aggregate_proof.go b/beacon-chain/sync/subscriber_beacon_aggregate_proof.go index 7e6609325983..84cb40072915 100644 --- a/beacon-chain/sync/subscriber_beacon_aggregate_proof.go +++ b/beacon-chain/sync/subscriber_beacon_aggregate_proof.go @@ -36,9 +36,7 @@ func (r *Service) beaconAggregateProofSubscriber(ctx context.Context, msg proto. // An unaggregated attestation can make it here. It’s valid, the aggregator it just itself, although it means poor performance for the subnet. if !helpers.IsAggregated(a.Message.Aggregate) { - if err := r.attPool.SaveUnaggregatedAttestation(a.Message.Aggregate); err != nil { - return err - } + return r.attPool.SaveUnaggregatedAttestation(a.Message.Aggregate) } return r.attPool.SaveAggregatedAttestation(a.Message.Aggregate) diff --git a/beacon-chain/sync/subscriber_beacon_aggregate_proof_test.go b/beacon-chain/sync/subscriber_beacon_aggregate_proof_test.go index 7a0e2b3f8437..60bc7314e07f 100644 --- a/beacon-chain/sync/subscriber_beacon_aggregate_proof_test.go +++ b/beacon-chain/sync/subscriber_beacon_aggregate_proof_test.go @@ -12,7 +12,7 @@ import ( "github.com/prysmaticlabs/prysm/beacon-chain/operations/attestations" ) -func TestBeaconAggregateProofSubscriber_CanSave(t *testing.T) { +func TestBeaconAggregateProofSubscriber_CanSaveAggregatedAttestation(t *testing.T) { c, err := lru.New(10) if err != nil { t.Fatal(err) @@ -32,3 +32,24 @@ func TestBeaconAggregateProofSubscriber_CanSave(t *testing.T) { t.Error("Did not save aggregated attestation") } } + +func TestBeaconAggregateProofSubscriber_CanSaveUnaggregatedAttestation(t *testing.T) { + c, err := lru.New(10) + if err != nil { + t.Fatal(err) + } + r := &Service{ + attPool: attestations.NewPool(), + seenAttestationCache: c, + attestationNotifier: (&mock.ChainService{}).OperationNotifier(), + } + + a := ðpb.SignedAggregateAttestationAndProof{Message: ðpb.AggregateAttestationAndProof{Aggregate: ðpb.Attestation{Data: ðpb.AttestationData{Target: ðpb.Checkpoint{}}, AggregationBits: bitfield.Bitlist{0x03}}, AggregatorIndex: 100}} + if err := r.beaconAggregateProofSubscriber(context.Background(), a); err != nil { + t.Fatal(err) + } + + if !reflect.DeepEqual(r.attPool.UnaggregatedAttestations(), []*ethpb.Attestation{a.Message.Aggregate}) { + t.Error("Did not save unaggregated attestation") + } +}