From 731cc0bd44460b1ff17389a9c5d987b0da82f54d Mon Sep 17 00:00:00 2001 From: terence tsao Date: Wed, 19 Feb 2020 06:46:30 -0800 Subject: [PATCH] Handing pending atts if they dont have the state (#4904) --- beacon-chain/sync/pending_attestations_queue.go | 4 ++-- beacon-chain/sync/pending_attestations_queue_test.go | 5 +++++ .../subscriber_committee_index_beacon_attestation_test.go | 5 +++++ beacon-chain/sync/validate_aggregate_proof.go | 6 ++++-- beacon-chain/sync/validate_aggregate_proof_test.go | 6 ++++++ .../sync/validate_committee_index_beacon_attestation.go | 6 ++++-- .../validate_committee_index_beacon_attestation_test.go | 5 +++++ 7 files changed, 31 insertions(+), 6 deletions(-) diff --git a/beacon-chain/sync/pending_attestations_queue.go b/beacon-chain/sync/pending_attestations_queue.go index cf31b82df..d5188d95d 100644 --- a/beacon-chain/sync/pending_attestations_queue.go +++ b/beacon-chain/sync/pending_attestations_queue.go @@ -60,8 +60,8 @@ func (s *Service) processPendingAtts(ctx context.Context) error { s.pendingAttsLock.RLock() attestations := s.blkRootToPendingAtts[bRoot] s.pendingAttsLock.RUnlock() - // Has the pending attestation's missing block arrived yet? - if s.db.HasBlock(ctx, bRoot) { + // Has the pending attestation's missing block arrived and the node processed block yet? + if s.db.HasBlock(ctx, bRoot) && s.db.HasState(ctx, bRoot) { numberOfBlocksRecoveredFromAtt.Inc() for _, att := range attestations { // The pending attestations can arrive in both aggregated and unaggregated forms, diff --git a/beacon-chain/sync/pending_attestations_queue_test.go b/beacon-chain/sync/pending_attestations_queue_test.go index 90c5378a2..f9eefecf7 100644 --- a/beacon-chain/sync/pending_attestations_queue_test.go +++ b/beacon-chain/sync/pending_attestations_queue_test.go @@ -16,6 +16,7 @@ import ( "github.com/prysmaticlabs/prysm/beacon-chain/operations/attestations" "github.com/prysmaticlabs/prysm/beacon-chain/p2p/peers" p2ptest "github.com/prysmaticlabs/prysm/beacon-chain/p2p/testing" + beaconstate "github.com/prysmaticlabs/prysm/beacon-chain/state" pb "github.com/prysmaticlabs/prysm/proto/beacon/p2p/v1" "github.com/prysmaticlabs/prysm/shared/attestationutil" "github.com/prysmaticlabs/prysm/shared/bls" @@ -77,7 +78,9 @@ func TestProcessPendingAtts_HasBlockSaveUnAggregatedAtt(t *testing.T) { b := ðpb.SignedBeaconBlock{Block: ðpb.BeaconBlock{}} r32, _ := ssz.HashTreeRoot(b.Block) + s, _ := beaconstate.InitializeFromProto(&pb.BeaconState{}) r.db.SaveBlock(context.Background(), b) + r.db.SaveState(context.Background(), s, r32) r.blkRootToPendingAtts[r32] = []*ethpb.AggregateAttestationAndProof{a} if err := r.processPendingAtts(context.Background()); err != nil { @@ -172,6 +175,8 @@ func TestProcessPendingAtts_HasBlockSaveAggregatedAtt(t *testing.T) { sb = ðpb.SignedBeaconBlock{Block: ðpb.BeaconBlock{}} r32, _ := ssz.HashTreeRoot(sb.Block) r.db.SaveBlock(context.Background(), sb) + s, _ := beaconstate.InitializeFromProto(&pb.BeaconState{}) + r.db.SaveState(context.Background(), s, r32) r.blkRootToPendingAtts[r32] = []*ethpb.AggregateAttestationAndProof{aggregateAndProof} if err := r.processPendingAtts(context.Background()); err != nil { diff --git a/beacon-chain/sync/subscriber_committee_index_beacon_attestation_test.go b/beacon-chain/sync/subscriber_committee_index_beacon_attestation_test.go index 135e78eca..b611bf061 100644 --- a/beacon-chain/sync/subscriber_committee_index_beacon_attestation_test.go +++ b/beacon-chain/sync/subscriber_committee_index_beacon_attestation_test.go @@ -14,7 +14,9 @@ import ( dbtest "github.com/prysmaticlabs/prysm/beacon-chain/db/testing" "github.com/prysmaticlabs/prysm/beacon-chain/operations/attestations" p2ptest "github.com/prysmaticlabs/prysm/beacon-chain/p2p/testing" + beaconstate "github.com/prysmaticlabs/prysm/beacon-chain/state" mockSync "github.com/prysmaticlabs/prysm/beacon-chain/sync/initial-sync/testing" + pb "github.com/prysmaticlabs/prysm/proto/beacon/p2p/v1" "github.com/prysmaticlabs/prysm/shared/testutil" ) @@ -39,6 +41,9 @@ func TestService_committeeIndexBeaconAttestationSubscriber_ValidMessage(t *testi if err := db.SaveBlock(ctx, blk); err != nil { t.Fatal(err) } + savedState, _ := beaconstate.InitializeFromProto(&pb.BeaconState{}) + db.SaveState(context.Background(), savedState, root) + r := &Service{ attPool: attestations.NewPool(), chain: &mock.ChainService{ diff --git a/beacon-chain/sync/validate_aggregate_proof.go b/beacon-chain/sync/validate_aggregate_proof.go index f569b4f39..3a48aef19 100644 --- a/beacon-chain/sync/validate_aggregate_proof.go +++ b/beacon-chain/sync/validate_aggregate_proof.go @@ -127,8 +127,10 @@ func (r *Service) validateAggregatedAtt(ctx context.Context, a *ethpb.AggregateA } func (r *Service) validateBlockInAttestation(ctx context.Context, a *ethpb.AggregateAttestationAndProof) bool { - // Verify the block being voted is in DB. The block should have passed validation if it's in the DB. - if !r.db.HasBlock(ctx, bytesutil.ToBytes32(a.Aggregate.Data.BeaconBlockRoot)) { + // Verify the block being voted and the processed state is in DB. The block should have passed validation if it's in the DB. + hasState := r.db.HasState(ctx, bytesutil.ToBytes32(a.Aggregate.Data.BeaconBlockRoot)) + hasBlock := r.db.HasBlock(ctx, bytesutil.ToBytes32(a.Aggregate.Data.BeaconBlockRoot)) + if !(hasState && hasBlock) { // A node doesn't have the block, it'll request from peer while saving the pending attestation to a queue. r.savePendingAtt(a) return false diff --git a/beacon-chain/sync/validate_aggregate_proof_test.go b/beacon-chain/sync/validate_aggregate_proof_test.go index c4884d2d9..daa40e85f 100644 --- a/beacon-chain/sync/validate_aggregate_proof_test.go +++ b/beacon-chain/sync/validate_aggregate_proof_test.go @@ -19,7 +19,9 @@ import ( "github.com/prysmaticlabs/prysm/beacon-chain/operations/attestations" "github.com/prysmaticlabs/prysm/beacon-chain/p2p" p2ptest "github.com/prysmaticlabs/prysm/beacon-chain/p2p/testing" + beaconstate "github.com/prysmaticlabs/prysm/beacon-chain/state" mockSync "github.com/prysmaticlabs/prysm/beacon-chain/sync/initial-sync/testing" + pb "github.com/prysmaticlabs/prysm/proto/beacon/p2p/v1" "github.com/prysmaticlabs/prysm/shared/attestationutil" "github.com/prysmaticlabs/prysm/shared/bls" "github.com/prysmaticlabs/prysm/shared/params" @@ -165,6 +167,8 @@ func TestValidateAggregateAndProof_NotWithinSlotRange(t *testing.T) { b := ðpb.SignedBeaconBlock{Block: ðpb.BeaconBlock{}} db.SaveBlock(context.Background(), b) root, _ := ssz.HashTreeRoot(b.Block) + s, _ := beaconstate.InitializeFromProto(&pb.BeaconState{}) + db.SaveState(context.Background(), s, root) aggBits := bitfield.NewBitlist(3) aggBits.SetBitAt(0, true) @@ -305,6 +309,8 @@ func TestValidateAggregateAndProof_CanValidate(t *testing.T) { b := ðpb.SignedBeaconBlock{Block: ðpb.BeaconBlock{}} db.SaveBlock(context.Background(), b) root, _ := ssz.HashTreeRoot(b.Block) + s, _ := beaconstate.InitializeFromProto(&pb.BeaconState{}) + db.SaveState(context.Background(), s, root) aggBits := bitfield.NewBitlist(3) aggBits.SetBitAt(0, true) diff --git a/beacon-chain/sync/validate_committee_index_beacon_attestation.go b/beacon-chain/sync/validate_committee_index_beacon_attestation.go index ecd98426b..6f7f09892 100644 --- a/beacon-chain/sync/validate_committee_index_beacon_attestation.go +++ b/beacon-chain/sync/validate_committee_index_beacon_attestation.go @@ -73,8 +73,10 @@ func (s *Service) validateCommitteeIndexBeaconAttestation(ctx context.Context, p return false } - // Verify the block being voted is in DB. The block should have passed validation if it's in the DB. - if !s.db.HasBlock(ctx, bytesutil.ToBytes32(att.Data.BeaconBlockRoot)) { + // Verify the block being voted and the processed state is in DB and. The block should have passed validation if it's in the DB. + hasState := s.db.HasState(ctx, bytesutil.ToBytes32(att.Data.BeaconBlockRoot)) + hasBlock := s.db.HasBlock(ctx, bytesutil.ToBytes32(att.Data.BeaconBlockRoot)) + if !(hasState && hasBlock) { // A node doesn't have the block, it'll request from peer while saving the pending attestation to a queue. s.savePendingAtt(ð.AggregateAttestationAndProof{Aggregate: att}) return false diff --git a/beacon-chain/sync/validate_committee_index_beacon_attestation_test.go b/beacon-chain/sync/validate_committee_index_beacon_attestation_test.go index 34e610db5..c4474a21d 100644 --- a/beacon-chain/sync/validate_committee_index_beacon_attestation_test.go +++ b/beacon-chain/sync/validate_committee_index_beacon_attestation_test.go @@ -14,7 +14,9 @@ import ( mockChain "github.com/prysmaticlabs/prysm/beacon-chain/blockchain/testing" dbtest "github.com/prysmaticlabs/prysm/beacon-chain/db/testing" p2ptest "github.com/prysmaticlabs/prysm/beacon-chain/p2p/testing" + beaconstate "github.com/prysmaticlabs/prysm/beacon-chain/state" mockSync "github.com/prysmaticlabs/prysm/beacon-chain/sync/initial-sync/testing" + pb "github.com/prysmaticlabs/prysm/proto/beacon/p2p/v1" "github.com/prysmaticlabs/prysm/shared/params" ) @@ -49,6 +51,9 @@ func TestService_validateCommitteeIndexBeaconAttestation(t *testing.T) { t.Fatal(err) } + savedState, _ := beaconstate.InitializeFromProto(&pb.BeaconState{}) + db.SaveState(context.Background(), savedState, validBlockRoot) + tests := []struct { name string msg *ethpb.Attestation