From b631be60400dda9e4e1be46f2c12ba1114280e6f Mon Sep 17 00:00:00 2001 From: Raul Jordan Date: Wed, 31 Aug 2022 15:24:00 -0400 Subject: [PATCH 01/15] atts performance and blocks --- beacon-chain/sync/metrics.go | 41 +++++++++++++++++++ beacon-chain/sync/validate_aggregate_proof.go | 6 +++ .../sync/validate_beacon_attestation.go | 6 +++ beacon-chain/sync/validate_beacon_blocks.go | 22 ++++++---- 4 files changed, 66 insertions(+), 9 deletions(-) diff --git a/beacon-chain/sync/metrics.go b/beacon-chain/sync/metrics.go index e56528805073..e1c5acec72cf 100644 --- a/beacon-chain/sync/metrics.go +++ b/beacon-chain/sync/metrics.go @@ -89,6 +89,47 @@ var ( Buckets: []float64{250, 500, 1000, 1500, 2000, 4000, 8000, 16000}, }, ) + + // Attestation processing errors and successes. + unaggregatedAttsProcessedCount = promauto.NewCounter(prometheus.CounterOpts{ + Name: "gossip_unaggregated_attestations_processed_total", + Help: "Number of unaggregated attestations processed from gossipsub", + }) + unaggregatedAttsFailedProcessingCount = promauto.NewCounter(prometheus.CounterOpts{ + Name: "gossip_unaggregated_attestations_failed_processing_total", + Help: "Number of unaggregated attestations that fail processing from gossipsub", + }) + aggregateAttsProcessedCount = promauto.NewCounter(prometheus.CounterOpts{ + Name: "gossip_aggregate_attestations_processed_total", + Help: "Number of aggregate attestations processed from gossipsub", + }) + aggregateAttsFailedProcessingCount = promauto.NewCounter(prometheus.CounterOpts{ + Name: "gossip_aggregate_attestations_failed_processing_total", + Help: "Number of aggregate attestations that fail processing from gossipsub", + }) + aggregateAttsFailedProcessingCount = promauto.NewCounter(prometheus.CounterOpts{ + Name: "gossip_aggregate_attestations_failed_processing_total", + Help: "Number of aggregate attestations that fail processing from gossipsub", + }) + // Attestation and block gossip verification performance. + unaggregatedAttestationVerificationGossipSummary = promauto.NewSummary( + prometheus.SummaryOpts{ + Name: "gossip_unaggregate_attestation_verification_milliseconds", + Help: "Time to verify gossiped, unaggregated attestations", + }, + ) + aggregateAttestationVerificationGossipSummary = promauto.NewSummary( + prometheus.SummaryOpts{ + Name: "gossip_aggregate_attestation_verification_milliseconds", + Help: "Time to verify gossiped attestations", + }, + ) + blockVerificationGossipSummary = promauto.NewSummary( + prometheus.SummaryOpts{ + Name: "gossip_block_verification_milliseconds", + Help: "Time to verify gossiped blocks", + }, + ) ) func (s *Service) updateMetrics() { diff --git a/beacon-chain/sync/validate_aggregate_proof.go b/beacon-chain/sync/validate_aggregate_proof.go index 677873b641e2..0d83f9f961b0 100644 --- a/beacon-chain/sync/validate_aggregate_proof.go +++ b/beacon-chain/sync/validate_aggregate_proof.go @@ -3,6 +3,7 @@ package sync import ( "context" "fmt" + "time" "github.com/libp2p/go-libp2p-core/peer" pubsub "github.com/libp2p/go-libp2p-pubsub" @@ -20,6 +21,7 @@ import ( "github.com/prysmaticlabs/prysm/v3/encoding/bytesutil" "github.com/prysmaticlabs/prysm/v3/monitoring/tracing" ethpb "github.com/prysmaticlabs/prysm/v3/proto/prysm/v1alpha1" + prysmTime "github.com/prysmaticlabs/prysm/v3/time" "github.com/prysmaticlabs/prysm/v3/time/slots" "go.opencensus.io/trace" ) @@ -27,6 +29,7 @@ import ( // validateAggregateAndProof verifies the aggregated signature and the selection proof is valid before forwarding to the // network and downstream services. func (s *Service) validateAggregateAndProof(ctx context.Context, pid peer.ID, msg *pubsub.Message) (pubsub.ValidationResult, error) { + receivedTime := prysmTime.Now() if pid == s.cfg.p2p.PeerID() { return pubsub.ValidationAccept, nil } @@ -114,6 +117,9 @@ func (s *Service) validateAggregateAndProof(ctx context.Context, pid peer.ID, ms msg.ValidatorData = m + ms := prysmTime.Now().Sub(receivedTime) / time.Millisecond + aggregateAttestationVerificationGossipSummary.Observe(float64(ms)) + return pubsub.ValidationAccept, nil } diff --git a/beacon-chain/sync/validate_beacon_attestation.go b/beacon-chain/sync/validate_beacon_attestation.go index c92d5043f720..4d1005cdac7c 100644 --- a/beacon-chain/sync/validate_beacon_attestation.go +++ b/beacon-chain/sync/validate_beacon_attestation.go @@ -5,6 +5,7 @@ import ( "fmt" "reflect" "strings" + "time" "github.com/libp2p/go-libp2p-core/peer" pubsub "github.com/libp2p/go-libp2p-pubsub" @@ -21,6 +22,7 @@ import ( "github.com/prysmaticlabs/prysm/v3/monitoring/tracing" eth "github.com/prysmaticlabs/prysm/v3/proto/prysm/v1alpha1" "github.com/prysmaticlabs/prysm/v3/proto/prysm/v1alpha1/attestation" + prysmTime "github.com/prysmaticlabs/prysm/v3/time" "github.com/prysmaticlabs/prysm/v3/time/slots" "go.opencensus.io/trace" ) @@ -32,6 +34,7 @@ import ( // - attestation.data.slot is within the last ATTESTATION_PROPAGATION_SLOT_RANGE slots (attestation.data.slot + ATTESTATION_PROPAGATION_SLOT_RANGE >= current_slot >= attestation.data.slot). // - The signature of attestation is valid. func (s *Service) validateCommitteeIndexBeaconAttestation(ctx context.Context, pid peer.ID, msg *pubsub.Message) (pubsub.ValidationResult, error) { + receivedTime := prysmTime.Now() if pid == s.cfg.p2p.PeerID() { return pubsub.ValidationAccept, nil } @@ -164,6 +167,9 @@ func (s *Service) validateCommitteeIndexBeaconAttestation(ctx context.Context, p msg.ValidatorData = att + ms := prysmTime.Now().Sub(receivedTime) / time.Millisecond + unaggregatedAttestationVerificationGossipSummary.Observe(float64(ms)) + return pubsub.ValidationAccept, nil } diff --git a/beacon-chain/sync/validate_beacon_blocks.go b/beacon-chain/sync/validate_beacon_blocks.go index 8955503ece5c..ab9878e39a29 100644 --- a/beacon-chain/sync/validate_beacon_blocks.go +++ b/beacon-chain/sync/validate_beacon_blocks.go @@ -204,6 +204,9 @@ func (s *Service) validateBeaconBlockPubSub(ctx context.Context, pid peer.ID, ms "proposerIndex": blk.Block().ProposerIndex(), "graffiti": string(blk.Block().Body().Graffiti()), }).Debug("Received block") + + ms := prysmTime.Now().Sub(receivedTime) / time.Millisecond + blockVerificationGossipSummary.Observe(float64(ms)) return pubsub.ValidationAccept, nil } @@ -253,16 +256,17 @@ func (s *Service) validateBeaconBlock(ctx context.Context, blk interfaces.Signed // validateBellatrixBeaconBlock validates the block for the Bellatrix fork. // spec code: -// If the execution is enabled for the block -- i.e. is_execution_enabled(state, block.body) then validate the following: -// [REJECT] The block's execution payload timestamp is correct with respect to the slot -- -// i.e. execution_payload.timestamp == compute_timestamp_at_slot(state, block.slot). // -// If exection_payload verification of block's parent by an execution node is not complete: -// [REJECT] The block's parent (defined by block.parent_root) passes all validation (excluding execution -// node verification of the block.body.execution_payload). -// otherwise: -// [IGNORE] The block's parent (defined by block.parent_root) passes all validation (including execution -// node verification of the block.body.execution_payload). +// If the execution is enabled for the block -- i.e. is_execution_enabled(state, block.body) then validate the following: +// [REJECT] The block's execution payload timestamp is correct with respect to the slot -- +// i.e. execution_payload.timestamp == compute_timestamp_at_slot(state, block.slot). +// +// If exection_payload verification of block's parent by an execution node is not complete: +// [REJECT] The block's parent (defined by block.parent_root) passes all validation (excluding execution +// node verification of the block.body.execution_payload). +// otherwise: +// [IGNORE] The block's parent (defined by block.parent_root) passes all validation (including execution +// node verification of the block.body.execution_payload). func (s *Service) validateBellatrixBeaconBlock(ctx context.Context, parentState state.BeaconState, blk interfaces.BeaconBlock) error { // Error if block and state are not the same version if parentState.Version() != blk.Version() { From 095c28941ea3a7c25b5b12b31d9ebe11c3fccce1 Mon Sep 17 00:00:00 2001 From: Raul Jordan Date: Wed, 31 Aug 2022 18:09:38 -0400 Subject: [PATCH 02/15] idiomatic observe --- beacon-chain/sync/validate_aggregate_proof.go | 4 +--- beacon-chain/sync/validate_beacon_attestation.go | 4 +--- beacon-chain/sync/validate_beacon_blocks.go | 3 +-- 3 files changed, 3 insertions(+), 8 deletions(-) diff --git a/beacon-chain/sync/validate_aggregate_proof.go b/beacon-chain/sync/validate_aggregate_proof.go index 0d83f9f961b0..eb19709dbc16 100644 --- a/beacon-chain/sync/validate_aggregate_proof.go +++ b/beacon-chain/sync/validate_aggregate_proof.go @@ -3,7 +3,6 @@ package sync import ( "context" "fmt" - "time" "github.com/libp2p/go-libp2p-core/peer" pubsub "github.com/libp2p/go-libp2p-pubsub" @@ -117,8 +116,7 @@ func (s *Service) validateAggregateAndProof(ctx context.Context, pid peer.ID, ms msg.ValidatorData = m - ms := prysmTime.Now().Sub(receivedTime) / time.Millisecond - aggregateAttestationVerificationGossipSummary.Observe(float64(ms)) + aggregateAttestationVerificationGossipSummary.Observe(float64(prysmTime.Since(receivedTime).Milliseconds())) return pubsub.ValidationAccept, nil } diff --git a/beacon-chain/sync/validate_beacon_attestation.go b/beacon-chain/sync/validate_beacon_attestation.go index 4d1005cdac7c..e99ca6ba1db2 100644 --- a/beacon-chain/sync/validate_beacon_attestation.go +++ b/beacon-chain/sync/validate_beacon_attestation.go @@ -5,7 +5,6 @@ import ( "fmt" "reflect" "strings" - "time" "github.com/libp2p/go-libp2p-core/peer" pubsub "github.com/libp2p/go-libp2p-pubsub" @@ -167,8 +166,7 @@ func (s *Service) validateCommitteeIndexBeaconAttestation(ctx context.Context, p msg.ValidatorData = att - ms := prysmTime.Now().Sub(receivedTime) / time.Millisecond - unaggregatedAttestationVerificationGossipSummary.Observe(float64(ms)) + unaggregatedAttestationVerificationGossipSummary.Observe(float64(prysmTime.Since(receivedTime).Milliseconds())) return pubsub.ValidationAccept, nil } diff --git a/beacon-chain/sync/validate_beacon_blocks.go b/beacon-chain/sync/validate_beacon_blocks.go index ab9878e39a29..7120b4f0d6f8 100644 --- a/beacon-chain/sync/validate_beacon_blocks.go +++ b/beacon-chain/sync/validate_beacon_blocks.go @@ -205,8 +205,7 @@ func (s *Service) validateBeaconBlockPubSub(ctx context.Context, pid peer.ID, ms "graffiti": string(blk.Block().Body().Graffiti()), }).Debug("Received block") - ms := prysmTime.Now().Sub(receivedTime) / time.Millisecond - blockVerificationGossipSummary.Observe(float64(ms)) + blockVerificationGossipSummary.Observe(float64(prysmTime.Since(receivedTime).Milliseconds())) return pubsub.ValidationAccept, nil } From 92c77d81291b9be9715d260437049409c9748b2b Mon Sep 17 00:00:00 2001 From: Raul Jordan Date: Wed, 31 Aug 2022 18:50:30 -0400 Subject: [PATCH 03/15] all attestation related errors --- beacon-chain/core/helpers/attestation.go | 82 +++++++++++-------- beacon-chain/core/helpers/metrics.go | 17 ++++ beacon-chain/sync/batch_verifier.go | 1 + beacon-chain/sync/metrics.go | 56 ++++++++++++- .../sync/subscriber_beacon_aggregate_proof.go | 12 ++- .../sync/subscriber_beacon_attestation.go | 6 +- beacon-chain/sync/validate_aggregate_proof.go | 25 +++++- .../sync/validate_beacon_attestation.go | 18 ++++ 8 files changed, 173 insertions(+), 44 deletions(-) create mode 100644 beacon-chain/core/helpers/metrics.go diff --git a/beacon-chain/core/helpers/attestation.go b/beacon-chain/core/helpers/attestation.go index dd321553c114..325bf16d40b4 100644 --- a/beacon-chain/core/helpers/attestation.go +++ b/beacon-chain/core/helpers/attestation.go @@ -51,10 +51,11 @@ func ValidateSlotTargetEpoch(data *ethpb.AttestationData) error { // committee count as an argument allows cheaper computation at run time. // // Spec pseudocode definition: -// def is_aggregator(state: BeaconState, slot: Slot, index: CommitteeIndex, slot_signature: BLSSignature) -> bool: -// committee = get_beacon_committee(state, slot, index) -// modulo = max(1, len(committee) // TARGET_AGGREGATORS_PER_COMMITTEE) -// return bytes_to_uint64(hash(slot_signature)[0:8]) % modulo == 0 +// +// def is_aggregator(state: BeaconState, slot: Slot, index: CommitteeIndex, slot_signature: BLSSignature) -> bool: +// committee = get_beacon_committee(state, slot, index) +// modulo = max(1, len(committee) // TARGET_AGGREGATORS_PER_COMMITTEE) +// return bytes_to_uint64(hash(slot_signature)[0:8]) % modulo == 0 func IsAggregator(committeeCount uint64, slotSig []byte) (bool, error) { modulo := uint64(1) if committeeCount/params.BeaconConfig().TargetAggregatorsPerCommittee > 1 { @@ -68,9 +69,10 @@ func IsAggregator(committeeCount uint64, slotSig []byte) (bool, error) { // AggregateSignature returns the aggregated signature of the input attestations. // // Spec pseudocode definition: -// def get_aggregate_signature(attestations: Sequence[Attestation]) -> BLSSignature: -// signatures = [attestation.signature for attestation in attestations] -// return bls.Aggregate(signatures) +// +// def get_aggregate_signature(attestations: Sequence[Attestation]) -> BLSSignature: +// signatures = [attestation.signature for attestation in attestations] +// return bls.Aggregate(signatures) func AggregateSignature(attestations []*ethpb.Attestation) (bls.Signature, error) { sigs := make([]bls.Signature, len(attestations)) var err error @@ -95,14 +97,15 @@ func IsAggregated(attestation *ethpb.Attestation) bool { // // Spec pseudocode definition: // def compute_subnet_for_attestation(committees_per_slot: uint64, slot: Slot, committee_index: CommitteeIndex) -> uint64: -// """ -// Compute the correct subnet for an attestation for Phase 0. -// Note, this mimics expected future behavior where attestations will be mapped to their shard subnet. -// """ -// slots_since_epoch_start = uint64(slot % SLOTS_PER_EPOCH) -// committees_since_epoch_start = committees_per_slot * slots_since_epoch_start // -// return uint64((committees_since_epoch_start + committee_index) % ATTESTATION_SUBNET_COUNT) +// """ +// Compute the correct subnet for an attestation for Phase 0. +// Note, this mimics expected future behavior where attestations will be mapped to their shard subnet. +// """ +// slots_since_epoch_start = uint64(slot % SLOTS_PER_EPOCH) +// committees_since_epoch_start = committees_per_slot * slots_since_epoch_start +// +// return uint64((committees_since_epoch_start + committee_index) % ATTESTATION_SUBNET_COUNT) func ComputeSubnetForAttestation(activeValCount uint64, att *ethpb.Attestation) uint64 { return ComputeSubnetFromCommitteeAndSlot(activeValCount, att.Data.CommitteeIndex, att.Data.Slot) } @@ -112,14 +115,15 @@ func ComputeSubnetForAttestation(activeValCount uint64, att *ethpb.Attestation) // // Spec pseudocode definition: // def compute_subnet_for_attestation(committees_per_slot: uint64, slot: Slot, committee_index: CommitteeIndex) -> uint64: -// """ -// Compute the correct subnet for an attestation for Phase 0. -// Note, this mimics expected future behavior where attestations will be mapped to their shard subnet. -// """ -// slots_since_epoch_start = uint64(slot % SLOTS_PER_EPOCH) -// committees_since_epoch_start = committees_per_slot * slots_since_epoch_start // -// return uint64((committees_since_epoch_start + committee_index) % ATTESTATION_SUBNET_COUNT) +// """ +// Compute the correct subnet for an attestation for Phase 0. +// Note, this mimics expected future behavior where attestations will be mapped to their shard subnet. +// """ +// slots_since_epoch_start = uint64(slot % SLOTS_PER_EPOCH) +// committees_since_epoch_start = committees_per_slot * slots_since_epoch_start +// +// return uint64((committees_since_epoch_start + committee_index) % ATTESTATION_SUBNET_COUNT) func ComputeSubnetFromCommitteeAndSlot(activeValCount uint64, comIdx types.CommitteeIndex, attSlot types.Slot) uint64 { slotSinceStart := slots.SinceEpochStarts(attSlot) comCount := SlotCommitteeCount(activeValCount) @@ -133,13 +137,15 @@ func ComputeSubnetFromCommitteeAndSlot(activeValCount uint64, comIdx types.Commi // slots. // // Example: -// ATTESTATION_PROPAGATION_SLOT_RANGE = 5 -// clockDisparity = 24 seconds -// current_slot = 100 -// invalid_attestation_slot = 92 -// invalid_attestation_slot = 103 -// valid_attestation_slot = 98 -// valid_attestation_slot = 101 +// +// ATTESTATION_PROPAGATION_SLOT_RANGE = 5 +// clockDisparity = 24 seconds +// current_slot = 100 +// invalid_attestation_slot = 92 +// invalid_attestation_slot = 103 +// valid_attestation_slot = 98 +// valid_attestation_slot = 101 +// // In the attestation must be within the range of 95 to 102 in the example above. func ValidateAttestationTime(attSlot types.Slot, genesisTime time.Time, clockDisparity time.Duration) error { if err := slots.ValidateClock(attSlot, uint64(genesisTime.Unix())); err != nil { @@ -170,13 +176,19 @@ func ValidateAttestationTime(attSlot types.Slot, genesisTime time.Time, clockDis lowerBounds := lowerTime.Add(-clockDisparity) // Verify attestation slot within the time range. - if attTime.Before(lowerBounds) || attTime.After(upperBounds) { - return fmt.Errorf( - "attestation slot %d not within attestation propagation range of %d to %d (current slot)", - attSlot, - lowerBoundsSlot, - currentSlot, - ) + attError := fmt.Errorf( + "attestation slot %d not within attestation propagation range of %d to %d (current slot)", + attSlot, + lowerBoundsSlot, + currentSlot, + ) + if attTime.Before(lowerBounds) { + attReceivedTooEarlyCount.Inc() + return attError + } + if attTime.After(upperBounds) { + attReceivedTooLateCount.Inc() + return attError } return nil } diff --git a/beacon-chain/core/helpers/metrics.go b/beacon-chain/core/helpers/metrics.go new file mode 100644 index 000000000000..c0a70e7cd865 --- /dev/null +++ b/beacon-chain/core/helpers/metrics.go @@ -0,0 +1,17 @@ +package helpers + +import ( + "github.com/prometheus/client_golang/prometheus" + "github.com/prometheus/client_golang/prometheus/promauto" +) + +var ( + attReceivedTooEarlyCount = promauto.NewCounter(prometheus.CounterOpts{ + Name: "gossip_attestation_too_early_ignored_total", + Help: "Increased when a gossip attestation fails decoding", + }) + attReceivedTooLateCount = promauto.NewCounter(prometheus.CounterOpts{ + Name: "gossip_attestation_too_late_ignored_total", + Help: "Increased when a gossip attestation fails decoding", + }) +) diff --git a/beacon-chain/sync/batch_verifier.go b/beacon-chain/sync/batch_verifier.go index b9bf013a9db5..768e4ec483f0 100644 --- a/beacon-chain/sync/batch_verifier.go +++ b/beacon-chain/sync/batch_verifier.go @@ -73,6 +73,7 @@ func (s *Service) validateWithBatchVerifier(ctx context.Context, message string, if !verified { verErr := errors.Errorf("Verification of %s failed", message) tracing.AnnotateError(span, verErr) + sigVerificationErrorCount.Inc() return pubsub.ValidationReject, verErr } } diff --git a/beacon-chain/sync/metrics.go b/beacon-chain/sync/metrics.go index e1c5acec72cf..2da10b8ce84c 100644 --- a/beacon-chain/sync/metrics.go +++ b/beacon-chain/sync/metrics.go @@ -90,7 +90,12 @@ var ( }, ) - // Attestation processing errors and successes. + sigVerificationErrorCount = promauto.NewCounter(prometheus.CounterOpts{ + Name: "gossip_signature_verification_failed_total", + Help: "Number of gossip messages that failed signature verification", + }) + + // Attestation processing failures and successes. unaggregatedAttsProcessedCount = promauto.NewCounter(prometheus.CounterOpts{ Name: "gossip_unaggregated_attestations_processed_total", Help: "Number of unaggregated attestations processed from gossipsub", @@ -107,10 +112,53 @@ var ( Name: "gossip_aggregate_attestations_failed_processing_total", Help: "Number of aggregate attestations that fail processing from gossipsub", }) - aggregateAttsFailedProcessingCount = promauto.NewCounter(prometheus.CounterOpts{ - Name: "gossip_aggregate_attestations_failed_processing_total", - Help: "Number of aggregate attestations that fail processing from gossipsub", + + // Attestation processing granular error tracking. + attCannotDecodePubsub = promauto.NewCounter(prometheus.CounterOpts{ + Name: "gossip_attestation_failed_decode_total", + Help: "Increased when a gossip attestation fails decoding", + }) + attInvalidMessageType = promauto.NewCounter(prometheus.CounterOpts{ + Name: "gossip_attestation_invalid_message_type_total", + Help: "Increased when a gossip attestation is the wrong message", + }) + attNilMessage = promauto.NewCounter(prometheus.CounterOpts{ + Name: "gossip_attestation_nil_message_total", + Help: "Increased when a gossip attestation is nil", + }) + attWrongTargetEpoch = promauto.NewCounter(prometheus.CounterOpts{ + Name: "gossip_attestation_wrong_target_total", + Help: "Increased when a gossip attestation fails target check", }) + attBadBlockCount = promauto.NewCounter(prometheus.CounterOpts{ + Name: "gossip_attestation_bad_block_total", + Help: "Increased when a gossip attestation references a bad block", + }) + attBadLmdConsistencyCount = promauto.NewCounter(prometheus.CounterOpts{ + Name: "gossip_attestation_bad_lmd_consistency_total", + Help: "Increased when a gossip attestation has bad LMD GHOST consistency", + }) + attValidatorNotInCommitteeCount = promauto.NewCounter(prometheus.CounterOpts{ + Name: "gossip_attestation_validator_not_in_committee_total", + Help: "Increased when a gossip attestation's validator is not the right committee", + }) + attBadSelectionProof = promauto.NewCounter(prometheus.CounterOpts{ + Name: "gossip_attestation_bad_selection_proof_total", + Help: "Increased when a gossip attestation has a bad selection proof", + }) + attBadBitfieldLengthCount = promauto.NewCounter(prometheus.CounterOpts{ + Name: "gossip_attestation_bad_bitfield_length_total", + Help: "Increased when a gossip attestation has a bad bitfield length", + }) + attInvalidBitfieldCount = promauto.NewCounter(prometheus.CounterOpts{ + Name: "gossip_attestation_invalid_bitfield_total", + Help: "Increased when a gossip attestation has an invalid bitfield", + }) + attBadSignatureBatchCount = promauto.NewCounter(prometheus.CounterOpts{ + Name: "gossip_attestation_bad_signature_batch_total", + Help: "Increased when a gossip attestation has a bad signature batch", + }) + // Attestation and block gossip verification performance. unaggregatedAttestationVerificationGossipSummary = promauto.NewSummary( prometheus.SummaryOpts{ diff --git a/beacon-chain/sync/subscriber_beacon_aggregate_proof.go b/beacon-chain/sync/subscriber_beacon_aggregate_proof.go index 6b5c434ac99d..69c2d858cdf9 100644 --- a/beacon-chain/sync/subscriber_beacon_aggregate_proof.go +++ b/beacon-chain/sync/subscriber_beacon_aggregate_proof.go @@ -24,8 +24,16 @@ func (s *Service) beaconAggregateProofSubscriber(_ context.Context, msg proto.Me // 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) { - return s.cfg.attPool.SaveUnaggregatedAttestation(a.Message.Aggregate) + if err := s.cfg.attPool.SaveUnaggregatedAttestation(a.Message.Aggregate); err != nil { + return err + } + unaggregatedAttsProcessedCount.Inc() + return nil } - return s.cfg.attPool.SaveAggregatedAttestation(a.Message.Aggregate) + if err := s.cfg.attPool.SaveAggregatedAttestation(a.Message.Aggregate); err != nil { + return err + } + aggregateAttsProcessedCount.Inc() + return nil } diff --git a/beacon-chain/sync/subscriber_beacon_attestation.go b/beacon-chain/sync/subscriber_beacon_attestation.go index 0bc2dcc15cfa..4ca07d930108 100644 --- a/beacon-chain/sync/subscriber_beacon_attestation.go +++ b/beacon-chain/sync/subscriber_beacon_attestation.go @@ -33,7 +33,11 @@ func (s *Service) committeeIndexBeaconAttestationSubscriber(_ context.Context, m return nil } - return s.cfg.attPool.SaveUnaggregatedAttestation(a) + if err := s.cfg.attPool.SaveUnaggregatedAttestation(a); err != nil { + return err + } + unaggregatedAttsProcessedCount.Inc() + return nil } func (_ *Service) persistentSubnetIndices() []uint64 { diff --git a/beacon-chain/sync/validate_aggregate_proof.go b/beacon-chain/sync/validate_aggregate_proof.go index eb19709dbc16..aed7cf4322fe 100644 --- a/beacon-chain/sync/validate_aggregate_proof.go +++ b/beacon-chain/sync/validate_aggregate_proof.go @@ -45,16 +45,24 @@ func (s *Service) validateAggregateAndProof(ctx context.Context, pid peer.ID, ms raw, err := s.decodePubsubMessage(msg) if err != nil { tracing.AnnotateError(span, err) + attCannotDecodePubsub.Inc() + aggregateAttsFailedProcessingCount.Inc() return pubsub.ValidationReject, err } m, ok := raw.(*ethpb.SignedAggregateAttestationAndProof) if !ok { + attInvalidMessageType.Inc() + aggregateAttsFailedProcessingCount.Inc() return pubsub.ValidationReject, errors.Errorf("invalid message type: %T", raw) } if m.Message == nil { + attNilMessage.Inc() + aggregateAttsFailedProcessingCount.Inc() return pubsub.ValidationReject, errNilMessage } if err := helpers.ValidateNilAttestation(m.Message.Aggregate); err != nil { + attNilMessage.Inc() + aggregateAttsFailedProcessingCount.Inc() return pubsub.ValidationReject, err } // Do not process slot 0 aggregates. @@ -72,13 +80,18 @@ func (s *Service) validateAggregateAndProof(ctx context.Context, pid peer.ID, ms }) if err := helpers.ValidateSlotTargetEpoch(m.Message.Aggregate.Data); err != nil { + attWrongTargetEpoch.Inc() + aggregateAttsFailedProcessingCount.Inc() return pubsub.ValidationReject, err } // Attestation's slot is within ATTESTATION_PROPAGATION_SLOT_RANGE and early attestation // processing tolerance. - if err := helpers.ValidateAttestationTime(m.Message.Aggregate.Data.Slot, s.cfg.chain.GenesisTime(), - earlyAttestationProcessingTolerance); err != nil { + if err := helpers.ValidateAttestationTime( + m.Message.Aggregate.Data.Slot, + s.cfg.chain.GenesisTime(), + earlyAttestationProcessingTolerance, + ); err != nil { tracing.AnnotateError(span, err) return pubsub.ValidationIgnore, err } @@ -91,6 +104,8 @@ func (s *Service) validateAggregateAndProof(ctx context.Context, pid peer.ID, ms if s.hasBadBlock(bytesutil.ToBytes32(m.Message.Aggregate.Data.BeaconBlockRoot)) || s.hasBadBlock(bytesutil.ToBytes32(m.Message.Aggregate.Data.Target.Root)) || s.hasBadBlock(bytesutil.ToBytes32(m.Message.Aggregate.Data.Source.Root)) { + attBadBlockCount.Inc() + aggregateAttsFailedProcessingCount.Inc() return pubsub.ValidationReject, errors.New("bad block referenced in attestation data") } @@ -131,6 +146,8 @@ func (s *Service) validateAggregatedAtt(ctx context.Context, signed *ethpb.Signe // but it's invalid in the spirit of the protocol. Here we choose safety over profit. if err := s.cfg.chain.VerifyLmdFfgConsistency(ctx, signed.Message.Aggregate); err != nil { tracing.AnnotateError(span, err) + attBadLmdConsistencyCount.Inc() + aggregateAttsFailedProcessingCount.Inc() return pubsub.ValidationReject, err } @@ -164,6 +181,8 @@ func (s *Service) validateAggregatedAtt(ctx context.Context, signed *ethpb.Signe if err := validateIndexInCommittee(ctx, bs, signed.Message.Aggregate, signed.Message.AggregatorIndex); err != nil { wrappedErr := errors.Wrapf(err, "Could not validate index in committee") tracing.AnnotateError(span, wrappedErr) + attValidatorNotInCommitteeCount.Inc() + aggregateAttsFailedProcessingCount.Inc() return pubsub.ValidationReject, wrappedErr } @@ -172,6 +191,8 @@ func (s *Service) validateAggregatedAtt(ctx context.Context, signed *ethpb.Signe if err != nil { wrappedErr := errors.Wrapf(err, "Could not validate selection for validator %d", signed.Message.AggregatorIndex) tracing.AnnotateError(span, wrappedErr) + attBadSelectionProof.Inc() + aggregateAttsFailedProcessingCount.Inc() return pubsub.ValidationReject, wrappedErr } diff --git a/beacon-chain/sync/validate_beacon_attestation.go b/beacon-chain/sync/validate_beacon_attestation.go index e99ca6ba1db2..865d3d25df5d 100644 --- a/beacon-chain/sync/validate_beacon_attestation.go +++ b/beacon-chain/sync/validate_beacon_attestation.go @@ -53,15 +53,21 @@ func (s *Service) validateCommitteeIndexBeaconAttestation(ctx context.Context, p m, err := s.decodePubsubMessage(msg) if err != nil { tracing.AnnotateError(span, err) + attCannotDecodePubsub.Inc() + unaggregatedAttsFailedProcessingCount.Inc() return pubsub.ValidationReject, err } att, ok := m.(*eth.Attestation) if !ok { + attInvalidMessageType.Inc() + unaggregatedAttsFailedProcessingCount.Inc() return pubsub.ValidationReject, errWrongMessage } if err := helpers.ValidateNilAttestation(att); err != nil { + attNilMessage.Inc() + unaggregatedAttsFailedProcessingCount.Inc() return pubsub.ValidationReject, err } // Do not process slot 0 attestations. @@ -85,6 +91,8 @@ func (s *Service) validateCommitteeIndexBeaconAttestation(ctx context.Context, p return pubsub.ValidationIgnore, err } if err := helpers.ValidateSlotTargetEpoch(att.Data); err != nil { + attWrongTargetEpoch.Inc() + unaggregatedAttsFailedProcessingCount.Inc() return pubsub.ValidationReject, err } @@ -126,6 +134,8 @@ func (s *Service) validateCommitteeIndexBeaconAttestation(ctx context.Context, p if s.hasBadBlock(bytesutil.ToBytes32(att.Data.BeaconBlockRoot)) || s.hasBadBlock(bytesutil.ToBytes32(att.Data.Target.Root)) || s.hasBadBlock(bytesutil.ToBytes32(att.Data.Source.Root)) { + attBadBlockCount.Inc() + unaggregatedAttsFailedProcessingCount.Inc() return pubsub.ValidationReject, errors.New("attestation data references bad block root") } @@ -143,6 +153,8 @@ func (s *Service) validateCommitteeIndexBeaconAttestation(ctx context.Context, p } if err := s.cfg.chain.VerifyLmdFfgConsistency(ctx, att); err != nil { tracing.AnnotateError(span, err) + attBadLmdConsistencyCount.Inc() + unaggregatedAttsFailedProcessingCount.Inc() return pubsub.ValidationReject, err } @@ -213,6 +225,8 @@ func (s *Service) validateUnaggregatedAttWithState(ctx context.Context, a *eth.A // Verify number of aggregation bits matches the committee size. if err := helpers.VerifyBitfieldLength(a.AggregationBits, uint64(len(committee))); err != nil { + attBadBitfieldLengthCount.Inc() + unaggregatedAttsFailedProcessingCount.Inc() return pubsub.ValidationReject, err } @@ -220,12 +234,16 @@ func (s *Service) validateUnaggregatedAttWithState(ctx context.Context, a *eth.A // Note: The Ethereum Beacon chain spec suggests (len(get_attesting_indices(state, attestation.data, attestation.aggregation_bits)) == 1) // however this validation can be achieved without use of get_attesting_indices which is an O(n) lookup. if a.AggregationBits.Count() != 1 || a.AggregationBits.BitIndices()[0] >= len(committee) { + attInvalidBitfieldCount.Inc() + unaggregatedAttsFailedProcessingCount.Inc() return pubsub.ValidationReject, errors.New("attestation bitfield is invalid") } set, err := blocks.AttestationSignatureBatch(ctx, bs, []*eth.Attestation{a}) if err != nil { tracing.AnnotateError(span, err) + attBadSignatureBatchCount.Inc() + unaggregatedAttsFailedProcessingCount.Inc() return pubsub.ValidationReject, err } return s.validateWithBatchVerifier(ctx, "attestation", set) From 7d47155fcca458e02ff77a3e4c87986548b59c59 Mon Sep 17 00:00:00 2001 From: Raul Jordan Date: Wed, 31 Aug 2022 19:07:15 -0400 Subject: [PATCH 04/15] block metrics --- beacon-chain/blockchain/metrics.go | 8 ++++++++ beacon-chain/blockchain/process_block.go | 10 +++++++++- beacon-chain/state/stategen/metrics.go | 12 ++++++++++++ beacon-chain/state/stategen/replayer.go | 9 +++++---- 4 files changed, 34 insertions(+), 5 deletions(-) diff --git a/beacon-chain/blockchain/metrics.go b/beacon-chain/blockchain/metrics.go index ad8f9f7f9e0b..d55f707ea706 100644 --- a/beacon-chain/blockchain/metrics.go +++ b/beacon-chain/blockchain/metrics.go @@ -162,6 +162,14 @@ var ( Name: "missed_payload_id_filled_count", Help: "", }) + onBlockProcessingTime = promauto.NewSummary(prometheus.SummaryOpts{ + Name: "on_block_processing_milliseconds", + Help: "Total time in milliseconds to complete a call to onBlock()", + }) + stateTransitionProcessingTime = promauto.NewSummary(prometheus.SummaryOpts{ + Name: "state_transition_processing_milliseconds", + Help: "Total time to call a state transition in onBlock()", + }) ) // reportSlotMetrics reports slot related metrics. diff --git a/beacon-chain/blockchain/process_block.go b/beacon-chain/blockchain/process_block.go index 85b159add200..4595cb23c160 100644 --- a/beacon-chain/blockchain/process_block.go +++ b/beacon-chain/blockchain/process_block.go @@ -98,6 +98,7 @@ func (s *Service) onBlock(ctx context.Context, signed interfaces.SignedBeaconBlo if err := consensusblocks.BeaconBlockIsNil(signed); err != nil { return invalidBlock{error: err} } + startTime := time.Now() b := signed.Block() preState, err := s.getBlockPreState(ctx, b) @@ -115,10 +116,13 @@ func (s *Service) onBlock(ctx context.Context, signed interfaces.SignedBeaconBlo if err != nil { return err } + stateTransitionStartTime := time.Now() postState, err := transition.ExecuteStateTransition(ctx, preState, signed) if err != nil { return invalidBlock{error: err} } + stateTransitionProcessingTime.Observe(float64(time.Since(stateTransitionStartTime).Milliseconds())) + postStateVersion, postStateHeader, err := getStateVersionAndPayload(postState) if err != nil { return err @@ -262,7 +266,11 @@ func (s *Service) onBlock(ctx context.Context, signed interfaces.SignedBeaconBlo } defer reportAttestationInclusion(b) - return s.handleEpochBoundary(ctx, postState) + if err := s.handleEpochBoundary(ctx, postState); err != nil { + return err + } + onBlockProcessingTime.Observe(float64(time.Since(startTime).Milliseconds())) + return nil } func getStateVersionAndPayload(st state.BeaconState) (int, *enginev1.ExecutionPayloadHeader, error) { diff --git a/beacon-chain/state/stategen/metrics.go b/beacon-chain/state/stategen/metrics.go index 143b2517944d..bbdc4f05527f 100644 --- a/beacon-chain/state/stategen/metrics.go +++ b/beacon-chain/state/stategen/metrics.go @@ -13,4 +13,16 @@ var ( Buckets: []float64{64, 256, 1024, 2048, 4096}, }, ) + replayBlocksSummary = promauto.NewSummary( + prometheus.SummaryOpts{ + Name: "replay_blocks_milliseconds", + Help: "Time it took to replay blocks", + }, + ) + replayToSlotSummary = promauto.NewSummary( + prometheus.SummaryOpts{ + Name: "replay_to_slot_milliseconds", + Help: "Time it took to replay to slot", + }, + ) ) diff --git a/beacon-chain/state/stategen/replayer.go b/beacon-chain/state/stategen/replayer.go index 9fc17402838e..c1f0f93df3fd 100644 --- a/beacon-chain/state/stategen/replayer.go +++ b/beacon-chain/state/stategen/replayer.go @@ -119,6 +119,7 @@ func (rs *stateReplayer) ReplayBlocks(ctx context.Context) (state.BeaconState, e log.WithFields(logrus.Fields{ "duration": duration, }).Debug("Finished calling process_blocks on all blocks in ReplayBlocks") + replayBlocksSummary.Observe(float64(duration.Milliseconds())) return s, nil } @@ -150,14 +151,14 @@ func (rs *stateReplayer) ReplayToSlot(ctx context.Context, replayTo types.Slot) // err will be handled after the bookend log s, err = ReplayProcessSlots(ctx, s, replayTo) - + if err != nil { + return nil, errors.Wrap(err, fmt.Sprintf("ReplayToSlot failed to seek to slot %d after applying blocks", replayTo)) + } duration := time.Since(start) log.WithFields(logrus.Fields{ "duration": duration, }).Debug("time spent in process_slots") - if err != nil { - return nil, errors.Wrap(err, fmt.Sprintf("ReplayToSlot failed to seek to slot %d after applying blocks", replayTo)) - } + replayToSlotSummary.Observe(float64(duration.Milliseconds())) return s, nil } From 386c0e4473583f344845af50bba49c790cce1b1f Mon Sep 17 00:00:00 2001 From: Raul Jordan Date: Wed, 31 Aug 2022 19:24:24 -0400 Subject: [PATCH 05/15] db metrics --- beacon-chain/core/helpers/metrics.go | 8 ++++---- beacon-chain/db/kv/blocks.go | 14 ++++++++++++-- beacon-chain/db/kv/kv.go | 20 ++++++++++++++++++++ beacon-chain/db/kv/state.go | 24 +++++++++++++++++++----- 4 files changed, 55 insertions(+), 11 deletions(-) diff --git a/beacon-chain/core/helpers/metrics.go b/beacon-chain/core/helpers/metrics.go index c0a70e7cd865..a9358b6bcd83 100644 --- a/beacon-chain/core/helpers/metrics.go +++ b/beacon-chain/core/helpers/metrics.go @@ -7,11 +7,11 @@ import ( var ( attReceivedTooEarlyCount = promauto.NewCounter(prometheus.CounterOpts{ - Name: "gossip_attestation_too_early_ignored_total", - Help: "Increased when a gossip attestation fails decoding", + Name: "attestation_too_early_total", + Help: "Increased when an attestation is considered too early", }) attReceivedTooLateCount = promauto.NewCounter(prometheus.CounterOpts{ - Name: "gossip_attestation_too_late_ignored_total", - Help: "Increased when a gossip attestation fails decoding", + Name: "attestation_too_late_total", + Help: "Increased when an attestation is considered too late", }) ) diff --git a/beacon-chain/db/kv/blocks.go b/beacon-chain/db/kv/blocks.go index fcbdba229b15..ffb241e2c995 100644 --- a/beacon-chain/db/kv/blocks.go +++ b/beacon-chain/db/kv/blocks.go @@ -4,6 +4,7 @@ import ( "bytes" "context" "fmt" + "time" "github.com/ethereum/go-ethereum/common" "github.com/golang/snappy" @@ -35,6 +36,7 @@ func (s *Store) Block(ctx context.Context, blockRoot [32]byte) (interfaces.Signe if v, ok := s.blockCache.Get(string(blockRoot[:])); v != nil && ok { return v.(interfaces.SignedBeaconBlock), nil } + startTime := time.Now() var blk interfaces.SignedBeaconBlock err := s.db.View(func(tx *bolt.Tx) error { bkt := tx.Bucket(blocksBucket) @@ -46,6 +48,7 @@ func (s *Store) Block(ctx context.Context, blockRoot [32]byte) (interfaces.Signe blk, err = unmarshalBlock(ctx, enc) return err }) + blockReadingTime.Observe(float64(time.Since(startTime).Milliseconds())) return blk, err } @@ -119,6 +122,7 @@ func (s *Store) Blocks(ctx context.Context, f *filters.QueryFilter) ([]interface blocks := make([]interfaces.SignedBeaconBlock, 0) blockRoots := make([][32]byte, 0) + startTime := time.Now() err := s.db.View(func(tx *bolt.Tx) error { bkt := tx.Bucket(blocksBucket) @@ -138,6 +142,7 @@ func (s *Store) Blocks(ctx context.Context, f *filters.QueryFilter) ([]interface } return nil }) + blocksReadingTime.Observe(float64(time.Since(startTime).Milliseconds())) return blocks, blockRoots, err } @@ -276,6 +281,7 @@ func (s *Store) SaveBlock(ctx context.Context, signed interfaces.SignedBeaconBlo func (s *Store) SaveBlocks(ctx context.Context, blks []interfaces.SignedBeaconBlock) error { ctx, span := trace.StartSpan(ctx, "BeaconDB.SaveBlocks") defer span.End() + startTime := time.Now() // Performing marshaling, hashing, and indexing outside the bolt transaction // to minimize the time we hold the DB lock. @@ -296,7 +302,7 @@ func (s *Store) SaveBlocks(ctx context.Context, blks []interfaces.SignedBeaconBl indicesByBucket := createBlockIndicesFromBlock(ctx, blk.Block()) indicesForBlocks[i] = indicesByBucket } - return s.db.Update(func(tx *bolt.Tx) error { + if err := s.db.Update(func(tx *bolt.Tx) error { bkt := tx.Bucket(blocksBucket) for i, blk := range blks { if existingBlock := bkt.Get(blockRoots[i]); existingBlock != nil { @@ -321,7 +327,11 @@ func (s *Store) SaveBlocks(ctx context.Context, blks []interfaces.SignedBeaconBl } } return nil - }) + }); err != nil { + return err + } + blocksSavingTime.Observe(float64(time.Since(startTime).Milliseconds())) + return nil } // SaveHeadBlockRoot to the db. diff --git a/beacon-chain/db/kv/kv.go b/beacon-chain/db/kv/kv.go index 887ba975a13d..dd575b2f0fa8 100644 --- a/beacon-chain/db/kv/kv.go +++ b/beacon-chain/db/kv/kv.go @@ -55,6 +55,26 @@ var ( Name: "validator_entry_cache_delete_total", Help: "The total number of cache deletes on the validator entry cache.", }) + stateReadingTime = promauto.NewSummary(prometheus.SummaryOpts{ + Name: "db_beacon_state_reading_milliseconds", + Help: "Milliseconds it takes to read a beacon state from the DB", + }) + stateSavingTime = promauto.NewSummary(prometheus.SummaryOpts{ + Name: "db_beacon_state_saving_milliseconds", + Help: "Milliseconds it takes to save a beacon state to the DB", + }) + blockReadingTime = promauto.NewSummary(prometheus.SummaryOpts{ + Name: "db_beacon_block_reading_milliseconds", + Help: "Milliseconds it takes to read a beacon block from the DB", + }) + blocksReadingTime = promauto.NewSummary(prometheus.SummaryOpts{ + Name: "db_beacon_blocks_reading_milliseconds", + Help: "Milliseconds it takes to read beacon blocks from the DB", + }) + blocksSavingTime = promauto.NewSummary(prometheus.SummaryOpts{ + Name: "db_beacon_blocks_saving_milliseconds", + Help: "Milliseconds it takes to save beacon blocks to the DB", + }) ) // BlockCacheSize specifies 1000 slots worth of blocks cached, which diff --git a/beacon-chain/db/kv/state.go b/beacon-chain/db/kv/state.go index 3dc527f640db..6c4a472df677 100644 --- a/beacon-chain/db/kv/state.go +++ b/beacon-chain/db/kv/state.go @@ -20,6 +20,7 @@ import ( "github.com/prysmaticlabs/prysm/v3/encoding/bytesutil" "github.com/prysmaticlabs/prysm/v3/monitoring/tracing" ethpb "github.com/prysmaticlabs/prysm/v3/proto/prysm/v1alpha1" + "github.com/prysmaticlabs/prysm/v3/time" "github.com/prysmaticlabs/prysm/v3/time/slots" bolt "go.etcd.io/bbolt" "go.opencensus.io/trace" @@ -30,6 +31,7 @@ import ( func (s *Store) State(ctx context.Context, blockRoot [32]byte) (state.BeaconState, error) { ctx, span := trace.StartSpan(ctx, "BeaconDB.State") defer span.End() + startTime := time.Now() enc, err := s.stateBytes(ctx, blockRoot) if err != nil { return nil, err @@ -44,7 +46,12 @@ func (s *Store) State(ctx context.Context, blockRoot [32]byte) (state.BeaconStat return nil, valErr } - return s.unmarshalState(ctx, enc, valEntries) + st, err := s.unmarshalState(ctx, enc, valEntries) + if err != nil { + return nil, err + } + stateReadingTime.Observe(float64(time.Since(startTime).Milliseconds())) + return st, err } // StateOrError is just like State(), except it only returns a non-error response @@ -127,6 +134,7 @@ func (s *Store) SaveStates(ctx context.Context, states []state.ReadOnlyBeaconSta if states == nil { return errors.New("nil state") } + startTime := time.Now() multipleEncs := make([][]byte, len(states)) for i, st := range states { stateBytes, err := marshalState(ctx, st) @@ -136,7 +144,7 @@ func (s *Store) SaveStates(ctx context.Context, states []state.ReadOnlyBeaconSta multipleEncs[i] = stateBytes } - return s.db.Update(func(tx *bolt.Tx) error { + if err := s.db.Update(func(tx *bolt.Tx) error { bucket := tx.Bucket(stateBucket) for i, rt := range blockRoots { indicesByBucket := createStateIndicesFromStateSlot(ctx, states[i].Slot()) @@ -148,7 +156,11 @@ func (s *Store) SaveStates(ctx context.Context, states []state.ReadOnlyBeaconSta } } return nil - }) + }); err != nil { + return err + } + stateSavingTime.Observe(float64(time.Since(startTime).Milliseconds())) + return nil } type withValidators interface { @@ -760,8 +772,10 @@ func createStateIndicesFromStateSlot(ctx context.Context, slot types.Slot) map[s // Only following states would be kept: // 1.) state_slot % archived_interval == 0. (e.g. archived_interval=2048, states with slot 2048, 4096... etc) // 2.) archived_interval - archived_interval/3 < state_slot % archived_interval -// (e.g. archived_interval=2048, states with slots after 1365). -// This is to tolerate skip slots. Not every state lays on the boundary. +// +// (e.g. archived_interval=2048, states with slots after 1365). +// This is to tolerate skip slots. Not every state lays on the boundary. +// // 3.) state with current finalized root // 4.) unfinalized States func (s *Store) CleanUpDirtyStates(ctx context.Context, slotsPerArchivedPoint types.Slot) error { From d5ae59415be277769b94660bbd48d3284332ee48 Mon Sep 17 00:00:00 2001 From: Raul Jordan Date: Wed, 31 Aug 2022 19:25:48 -0400 Subject: [PATCH 06/15] metrics func --- beacon-chain/core/helpers/BUILD.bazel | 1 + beacon-chain/db/kv/BUILD.bazel | 1 + 2 files changed, 2 insertions(+) diff --git a/beacon-chain/core/helpers/BUILD.bazel b/beacon-chain/core/helpers/BUILD.bazel index aabf47075b90..6824c0cd5eb3 100644 --- a/beacon-chain/core/helpers/BUILD.bazel +++ b/beacon-chain/core/helpers/BUILD.bazel @@ -7,6 +7,7 @@ go_library( "beacon_committee.go", "block.go", "genesis.go", + "metrics.go", "randao.go", "rewards_penalties.go", "shuffle.go", diff --git a/beacon-chain/db/kv/BUILD.bazel b/beacon-chain/db/kv/BUILD.bazel index fb5d0a68af20..50916bddf1bc 100644 --- a/beacon-chain/db/kv/BUILD.bazel +++ b/beacon-chain/db/kv/BUILD.bazel @@ -57,6 +57,7 @@ go_library( "//monitoring/tracing:go_default_library", "//proto/prysm/v1alpha1:go_default_library", "//runtime/version:go_default_library", + "//time:go_default_library", "//time/slots:go_default_library", "@com_github_dgraph_io_ristretto//:go_default_library", "@com_github_ethereum_go_ethereum//common:go_default_library", From 02f86e751d2606c275ba2750d74b714d1a5d67b4 Mon Sep 17 00:00:00 2001 From: Raul Jordan Date: Wed, 31 Aug 2022 19:43:27 -0400 Subject: [PATCH 07/15] rem old metrics --- beacon-chain/sync/metrics.go | 12 ------------ beacon-chain/sync/validate_aggregate_proof.go | 8 -------- beacon-chain/sync/validate_beacon_attestation.go | 3 --- 3 files changed, 23 deletions(-) diff --git a/beacon-chain/sync/metrics.go b/beacon-chain/sync/metrics.go index 2da10b8ce84c..7f4737bb5d3e 100644 --- a/beacon-chain/sync/metrics.go +++ b/beacon-chain/sync/metrics.go @@ -114,18 +114,6 @@ var ( }) // Attestation processing granular error tracking. - attCannotDecodePubsub = promauto.NewCounter(prometheus.CounterOpts{ - Name: "gossip_attestation_failed_decode_total", - Help: "Increased when a gossip attestation fails decoding", - }) - attInvalidMessageType = promauto.NewCounter(prometheus.CounterOpts{ - Name: "gossip_attestation_invalid_message_type_total", - Help: "Increased when a gossip attestation is the wrong message", - }) - attNilMessage = promauto.NewCounter(prometheus.CounterOpts{ - Name: "gossip_attestation_nil_message_total", - Help: "Increased when a gossip attestation is nil", - }) attWrongTargetEpoch = promauto.NewCounter(prometheus.CounterOpts{ Name: "gossip_attestation_wrong_target_total", Help: "Increased when a gossip attestation fails target check", diff --git a/beacon-chain/sync/validate_aggregate_proof.go b/beacon-chain/sync/validate_aggregate_proof.go index aed7cf4322fe..dd403b20a27c 100644 --- a/beacon-chain/sync/validate_aggregate_proof.go +++ b/beacon-chain/sync/validate_aggregate_proof.go @@ -45,24 +45,16 @@ func (s *Service) validateAggregateAndProof(ctx context.Context, pid peer.ID, ms raw, err := s.decodePubsubMessage(msg) if err != nil { tracing.AnnotateError(span, err) - attCannotDecodePubsub.Inc() - aggregateAttsFailedProcessingCount.Inc() return pubsub.ValidationReject, err } m, ok := raw.(*ethpb.SignedAggregateAttestationAndProof) if !ok { - attInvalidMessageType.Inc() - aggregateAttsFailedProcessingCount.Inc() return pubsub.ValidationReject, errors.Errorf("invalid message type: %T", raw) } if m.Message == nil { - attNilMessage.Inc() - aggregateAttsFailedProcessingCount.Inc() return pubsub.ValidationReject, errNilMessage } if err := helpers.ValidateNilAttestation(m.Message.Aggregate); err != nil { - attNilMessage.Inc() - aggregateAttsFailedProcessingCount.Inc() return pubsub.ValidationReject, err } // Do not process slot 0 aggregates. diff --git a/beacon-chain/sync/validate_beacon_attestation.go b/beacon-chain/sync/validate_beacon_attestation.go index 865d3d25df5d..db9c4b26cdb7 100644 --- a/beacon-chain/sync/validate_beacon_attestation.go +++ b/beacon-chain/sync/validate_beacon_attestation.go @@ -53,20 +53,17 @@ func (s *Service) validateCommitteeIndexBeaconAttestation(ctx context.Context, p m, err := s.decodePubsubMessage(msg) if err != nil { tracing.AnnotateError(span, err) - attCannotDecodePubsub.Inc() unaggregatedAttsFailedProcessingCount.Inc() return pubsub.ValidationReject, err } att, ok := m.(*eth.Attestation) if !ok { - attInvalidMessageType.Inc() unaggregatedAttsFailedProcessingCount.Inc() return pubsub.ValidationReject, errWrongMessage } if err := helpers.ValidateNilAttestation(att); err != nil { - attNilMessage.Inc() unaggregatedAttsFailedProcessingCount.Inc() return pubsub.ValidationReject, err } From ec45c246250dc9423f1d45ab85e5f8b55a523eed Mon Sep 17 00:00:00 2001 From: Raul Jordan Date: Wed, 31 Aug 2022 19:47:18 -0400 Subject: [PATCH 08/15] naming --- beacon-chain/sync/metrics.go | 4 ++-- beacon-chain/sync/validate_aggregate_proof.go | 4 ++-- beacon-chain/sync/validate_beacon_attestation.go | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/beacon-chain/sync/metrics.go b/beacon-chain/sync/metrics.go index 7f4737bb5d3e..bb2ed7ee3631 100644 --- a/beacon-chain/sync/metrics.go +++ b/beacon-chain/sync/metrics.go @@ -114,7 +114,7 @@ var ( }) // Attestation processing granular error tracking. - attWrongTargetEpoch = promauto.NewCounter(prometheus.CounterOpts{ + attWrongTargetEpochCount = promauto.NewCounter(prometheus.CounterOpts{ Name: "gossip_attestation_wrong_target_total", Help: "Increased when a gossip attestation fails target check", }) @@ -130,7 +130,7 @@ var ( Name: "gossip_attestation_validator_not_in_committee_total", Help: "Increased when a gossip attestation's validator is not the right committee", }) - attBadSelectionProof = promauto.NewCounter(prometheus.CounterOpts{ + attBadSelectionProofCount = promauto.NewCounter(prometheus.CounterOpts{ Name: "gossip_attestation_bad_selection_proof_total", Help: "Increased when a gossip attestation has a bad selection proof", }) diff --git a/beacon-chain/sync/validate_aggregate_proof.go b/beacon-chain/sync/validate_aggregate_proof.go index dd403b20a27c..9ab0fb0822cd 100644 --- a/beacon-chain/sync/validate_aggregate_proof.go +++ b/beacon-chain/sync/validate_aggregate_proof.go @@ -72,7 +72,7 @@ func (s *Service) validateAggregateAndProof(ctx context.Context, pid peer.ID, ms }) if err := helpers.ValidateSlotTargetEpoch(m.Message.Aggregate.Data); err != nil { - attWrongTargetEpoch.Inc() + attWrongTargetEpochCount.Inc() aggregateAttsFailedProcessingCount.Inc() return pubsub.ValidationReject, err } @@ -183,7 +183,7 @@ func (s *Service) validateAggregatedAtt(ctx context.Context, signed *ethpb.Signe if err != nil { wrappedErr := errors.Wrapf(err, "Could not validate selection for validator %d", signed.Message.AggregatorIndex) tracing.AnnotateError(span, wrappedErr) - attBadSelectionProof.Inc() + attBadSelectionProofCount.Inc() aggregateAttsFailedProcessingCount.Inc() return pubsub.ValidationReject, wrappedErr } diff --git a/beacon-chain/sync/validate_beacon_attestation.go b/beacon-chain/sync/validate_beacon_attestation.go index db9c4b26cdb7..97df28331495 100644 --- a/beacon-chain/sync/validate_beacon_attestation.go +++ b/beacon-chain/sync/validate_beacon_attestation.go @@ -88,7 +88,7 @@ func (s *Service) validateCommitteeIndexBeaconAttestation(ctx context.Context, p return pubsub.ValidationIgnore, err } if err := helpers.ValidateSlotTargetEpoch(att.Data); err != nil { - attWrongTargetEpoch.Inc() + attWrongTargetEpochCount.Inc() unaggregatedAttsFailedProcessingCount.Inc() return pubsub.ValidationReject, err } From 03ced875e4c27f1a3d4cf86dd27de0e5ce46c12f Mon Sep 17 00:00:00 2001 From: Raul Jordan Date: Wed, 31 Aug 2022 20:14:45 -0400 Subject: [PATCH 09/15] rem metric --- beacon-chain/db/kv/blocks.go | 14 ++------------ beacon-chain/db/kv/kv.go | 12 ------------ 2 files changed, 2 insertions(+), 24 deletions(-) diff --git a/beacon-chain/db/kv/blocks.go b/beacon-chain/db/kv/blocks.go index ffb241e2c995..fcbdba229b15 100644 --- a/beacon-chain/db/kv/blocks.go +++ b/beacon-chain/db/kv/blocks.go @@ -4,7 +4,6 @@ import ( "bytes" "context" "fmt" - "time" "github.com/ethereum/go-ethereum/common" "github.com/golang/snappy" @@ -36,7 +35,6 @@ func (s *Store) Block(ctx context.Context, blockRoot [32]byte) (interfaces.Signe if v, ok := s.blockCache.Get(string(blockRoot[:])); v != nil && ok { return v.(interfaces.SignedBeaconBlock), nil } - startTime := time.Now() var blk interfaces.SignedBeaconBlock err := s.db.View(func(tx *bolt.Tx) error { bkt := tx.Bucket(blocksBucket) @@ -48,7 +46,6 @@ func (s *Store) Block(ctx context.Context, blockRoot [32]byte) (interfaces.Signe blk, err = unmarshalBlock(ctx, enc) return err }) - blockReadingTime.Observe(float64(time.Since(startTime).Milliseconds())) return blk, err } @@ -122,7 +119,6 @@ func (s *Store) Blocks(ctx context.Context, f *filters.QueryFilter) ([]interface blocks := make([]interfaces.SignedBeaconBlock, 0) blockRoots := make([][32]byte, 0) - startTime := time.Now() err := s.db.View(func(tx *bolt.Tx) error { bkt := tx.Bucket(blocksBucket) @@ -142,7 +138,6 @@ func (s *Store) Blocks(ctx context.Context, f *filters.QueryFilter) ([]interface } return nil }) - blocksReadingTime.Observe(float64(time.Since(startTime).Milliseconds())) return blocks, blockRoots, err } @@ -281,7 +276,6 @@ func (s *Store) SaveBlock(ctx context.Context, signed interfaces.SignedBeaconBlo func (s *Store) SaveBlocks(ctx context.Context, blks []interfaces.SignedBeaconBlock) error { ctx, span := trace.StartSpan(ctx, "BeaconDB.SaveBlocks") defer span.End() - startTime := time.Now() // Performing marshaling, hashing, and indexing outside the bolt transaction // to minimize the time we hold the DB lock. @@ -302,7 +296,7 @@ func (s *Store) SaveBlocks(ctx context.Context, blks []interfaces.SignedBeaconBl indicesByBucket := createBlockIndicesFromBlock(ctx, blk.Block()) indicesForBlocks[i] = indicesByBucket } - if err := s.db.Update(func(tx *bolt.Tx) error { + return s.db.Update(func(tx *bolt.Tx) error { bkt := tx.Bucket(blocksBucket) for i, blk := range blks { if existingBlock := bkt.Get(blockRoots[i]); existingBlock != nil { @@ -327,11 +321,7 @@ func (s *Store) SaveBlocks(ctx context.Context, blks []interfaces.SignedBeaconBl } } return nil - }); err != nil { - return err - } - blocksSavingTime.Observe(float64(time.Since(startTime).Milliseconds())) - return nil + }) } // SaveHeadBlockRoot to the db. diff --git a/beacon-chain/db/kv/kv.go b/beacon-chain/db/kv/kv.go index dd575b2f0fa8..93890844d4ab 100644 --- a/beacon-chain/db/kv/kv.go +++ b/beacon-chain/db/kv/kv.go @@ -63,18 +63,6 @@ var ( Name: "db_beacon_state_saving_milliseconds", Help: "Milliseconds it takes to save a beacon state to the DB", }) - blockReadingTime = promauto.NewSummary(prometheus.SummaryOpts{ - Name: "db_beacon_block_reading_milliseconds", - Help: "Milliseconds it takes to read a beacon block from the DB", - }) - blocksReadingTime = promauto.NewSummary(prometheus.SummaryOpts{ - Name: "db_beacon_blocks_reading_milliseconds", - Help: "Milliseconds it takes to read beacon blocks from the DB", - }) - blocksSavingTime = promauto.NewSummary(prometheus.SummaryOpts{ - Name: "db_beacon_blocks_saving_milliseconds", - Help: "Milliseconds it takes to save beacon blocks to the DB", - }) ) // BlockCacheSize specifies 1000 slots worth of blocks cached, which From d126b28ee18cb71d06bb5c90d3efb3e420fbf7f2 Mon Sep 17 00:00:00 2001 From: Raul Jordan Date: Wed, 31 Aug 2022 20:16:53 -0400 Subject: [PATCH 10/15] rem unneeded --- beacon-chain/sync/metrics.go | 23 ------------------- beacon-chain/sync/validate_aggregate_proof.go | 5 ---- .../sync/validate_beacon_attestation.go | 9 -------- 3 files changed, 37 deletions(-) diff --git a/beacon-chain/sync/metrics.go b/beacon-chain/sync/metrics.go index bb2ed7ee3631..7922d03a2e62 100644 --- a/beacon-chain/sync/metrics.go +++ b/beacon-chain/sync/metrics.go @@ -90,29 +90,6 @@ var ( }, ) - sigVerificationErrorCount = promauto.NewCounter(prometheus.CounterOpts{ - Name: "gossip_signature_verification_failed_total", - Help: "Number of gossip messages that failed signature verification", - }) - - // Attestation processing failures and successes. - unaggregatedAttsProcessedCount = promauto.NewCounter(prometheus.CounterOpts{ - Name: "gossip_unaggregated_attestations_processed_total", - Help: "Number of unaggregated attestations processed from gossipsub", - }) - unaggregatedAttsFailedProcessingCount = promauto.NewCounter(prometheus.CounterOpts{ - Name: "gossip_unaggregated_attestations_failed_processing_total", - Help: "Number of unaggregated attestations that fail processing from gossipsub", - }) - aggregateAttsProcessedCount = promauto.NewCounter(prometheus.CounterOpts{ - Name: "gossip_aggregate_attestations_processed_total", - Help: "Number of aggregate attestations processed from gossipsub", - }) - aggregateAttsFailedProcessingCount = promauto.NewCounter(prometheus.CounterOpts{ - Name: "gossip_aggregate_attestations_failed_processing_total", - Help: "Number of aggregate attestations that fail processing from gossipsub", - }) - // Attestation processing granular error tracking. attWrongTargetEpochCount = promauto.NewCounter(prometheus.CounterOpts{ Name: "gossip_attestation_wrong_target_total", diff --git a/beacon-chain/sync/validate_aggregate_proof.go b/beacon-chain/sync/validate_aggregate_proof.go index 9ab0fb0822cd..128e2aef4461 100644 --- a/beacon-chain/sync/validate_aggregate_proof.go +++ b/beacon-chain/sync/validate_aggregate_proof.go @@ -73,7 +73,6 @@ func (s *Service) validateAggregateAndProof(ctx context.Context, pid peer.ID, ms if err := helpers.ValidateSlotTargetEpoch(m.Message.Aggregate.Data); err != nil { attWrongTargetEpochCount.Inc() - aggregateAttsFailedProcessingCount.Inc() return pubsub.ValidationReject, err } @@ -97,7 +96,6 @@ func (s *Service) validateAggregateAndProof(ctx context.Context, pid peer.ID, ms s.hasBadBlock(bytesutil.ToBytes32(m.Message.Aggregate.Data.Target.Root)) || s.hasBadBlock(bytesutil.ToBytes32(m.Message.Aggregate.Data.Source.Root)) { attBadBlockCount.Inc() - aggregateAttsFailedProcessingCount.Inc() return pubsub.ValidationReject, errors.New("bad block referenced in attestation data") } @@ -139,7 +137,6 @@ func (s *Service) validateAggregatedAtt(ctx context.Context, signed *ethpb.Signe if err := s.cfg.chain.VerifyLmdFfgConsistency(ctx, signed.Message.Aggregate); err != nil { tracing.AnnotateError(span, err) attBadLmdConsistencyCount.Inc() - aggregateAttsFailedProcessingCount.Inc() return pubsub.ValidationReject, err } @@ -174,7 +171,6 @@ func (s *Service) validateAggregatedAtt(ctx context.Context, signed *ethpb.Signe wrappedErr := errors.Wrapf(err, "Could not validate index in committee") tracing.AnnotateError(span, wrappedErr) attValidatorNotInCommitteeCount.Inc() - aggregateAttsFailedProcessingCount.Inc() return pubsub.ValidationReject, wrappedErr } @@ -184,7 +180,6 @@ func (s *Service) validateAggregatedAtt(ctx context.Context, signed *ethpb.Signe wrappedErr := errors.Wrapf(err, "Could not validate selection for validator %d", signed.Message.AggregatorIndex) tracing.AnnotateError(span, wrappedErr) attBadSelectionProofCount.Inc() - aggregateAttsFailedProcessingCount.Inc() return pubsub.ValidationReject, wrappedErr } diff --git a/beacon-chain/sync/validate_beacon_attestation.go b/beacon-chain/sync/validate_beacon_attestation.go index 97df28331495..db0caf2d1730 100644 --- a/beacon-chain/sync/validate_beacon_attestation.go +++ b/beacon-chain/sync/validate_beacon_attestation.go @@ -53,18 +53,15 @@ func (s *Service) validateCommitteeIndexBeaconAttestation(ctx context.Context, p m, err := s.decodePubsubMessage(msg) if err != nil { tracing.AnnotateError(span, err) - unaggregatedAttsFailedProcessingCount.Inc() return pubsub.ValidationReject, err } att, ok := m.(*eth.Attestation) if !ok { - unaggregatedAttsFailedProcessingCount.Inc() return pubsub.ValidationReject, errWrongMessage } if err := helpers.ValidateNilAttestation(att); err != nil { - unaggregatedAttsFailedProcessingCount.Inc() return pubsub.ValidationReject, err } // Do not process slot 0 attestations. @@ -89,7 +86,6 @@ func (s *Service) validateCommitteeIndexBeaconAttestation(ctx context.Context, p } if err := helpers.ValidateSlotTargetEpoch(att.Data); err != nil { attWrongTargetEpochCount.Inc() - unaggregatedAttsFailedProcessingCount.Inc() return pubsub.ValidationReject, err } @@ -132,7 +128,6 @@ func (s *Service) validateCommitteeIndexBeaconAttestation(ctx context.Context, p s.hasBadBlock(bytesutil.ToBytes32(att.Data.Target.Root)) || s.hasBadBlock(bytesutil.ToBytes32(att.Data.Source.Root)) { attBadBlockCount.Inc() - unaggregatedAttsFailedProcessingCount.Inc() return pubsub.ValidationReject, errors.New("attestation data references bad block root") } @@ -151,7 +146,6 @@ func (s *Service) validateCommitteeIndexBeaconAttestation(ctx context.Context, p if err := s.cfg.chain.VerifyLmdFfgConsistency(ctx, att); err != nil { tracing.AnnotateError(span, err) attBadLmdConsistencyCount.Inc() - unaggregatedAttsFailedProcessingCount.Inc() return pubsub.ValidationReject, err } @@ -223,7 +217,6 @@ func (s *Service) validateUnaggregatedAttWithState(ctx context.Context, a *eth.A // Verify number of aggregation bits matches the committee size. if err := helpers.VerifyBitfieldLength(a.AggregationBits, uint64(len(committee))); err != nil { attBadBitfieldLengthCount.Inc() - unaggregatedAttsFailedProcessingCount.Inc() return pubsub.ValidationReject, err } @@ -232,7 +225,6 @@ func (s *Service) validateUnaggregatedAttWithState(ctx context.Context, a *eth.A // however this validation can be achieved without use of get_attesting_indices which is an O(n) lookup. if a.AggregationBits.Count() != 1 || a.AggregationBits.BitIndices()[0] >= len(committee) { attInvalidBitfieldCount.Inc() - unaggregatedAttsFailedProcessingCount.Inc() return pubsub.ValidationReject, errors.New("attestation bitfield is invalid") } @@ -240,7 +232,6 @@ func (s *Service) validateUnaggregatedAttWithState(ctx context.Context, a *eth.A if err != nil { tracing.AnnotateError(span, err) attBadSignatureBatchCount.Inc() - unaggregatedAttsFailedProcessingCount.Inc() return pubsub.ValidationReject, err } return s.validateWithBatchVerifier(ctx, "attestation", set) From 8c01ee31d73c50b959bbc3725222b08d289342f6 Mon Sep 17 00:00:00 2001 From: Raul Jordan Date: Wed, 31 Aug 2022 20:18:56 -0400 Subject: [PATCH 11/15] fix --- beacon-chain/sync/batch_verifier.go | 1 - beacon-chain/sync/subscriber_beacon_aggregate_proof.go | 2 -- beacon-chain/sync/subscriber_beacon_attestation.go | 1 - 3 files changed, 4 deletions(-) diff --git a/beacon-chain/sync/batch_verifier.go b/beacon-chain/sync/batch_verifier.go index 768e4ec483f0..b9bf013a9db5 100644 --- a/beacon-chain/sync/batch_verifier.go +++ b/beacon-chain/sync/batch_verifier.go @@ -73,7 +73,6 @@ func (s *Service) validateWithBatchVerifier(ctx context.Context, message string, if !verified { verErr := errors.Errorf("Verification of %s failed", message) tracing.AnnotateError(span, verErr) - sigVerificationErrorCount.Inc() return pubsub.ValidationReject, verErr } } diff --git a/beacon-chain/sync/subscriber_beacon_aggregate_proof.go b/beacon-chain/sync/subscriber_beacon_aggregate_proof.go index 69c2d858cdf9..63de7f7881df 100644 --- a/beacon-chain/sync/subscriber_beacon_aggregate_proof.go +++ b/beacon-chain/sync/subscriber_beacon_aggregate_proof.go @@ -27,13 +27,11 @@ func (s *Service) beaconAggregateProofSubscriber(_ context.Context, msg proto.Me if err := s.cfg.attPool.SaveUnaggregatedAttestation(a.Message.Aggregate); err != nil { return err } - unaggregatedAttsProcessedCount.Inc() return nil } if err := s.cfg.attPool.SaveAggregatedAttestation(a.Message.Aggregate); err != nil { return err } - aggregateAttsProcessedCount.Inc() return nil } diff --git a/beacon-chain/sync/subscriber_beacon_attestation.go b/beacon-chain/sync/subscriber_beacon_attestation.go index 4ca07d930108..341a54536144 100644 --- a/beacon-chain/sync/subscriber_beacon_attestation.go +++ b/beacon-chain/sync/subscriber_beacon_attestation.go @@ -36,7 +36,6 @@ func (s *Service) committeeIndexBeaconAttestationSubscriber(_ context.Context, m if err := s.cfg.attPool.SaveUnaggregatedAttestation(a); err != nil { return err } - unaggregatedAttsProcessedCount.Inc() return nil } From b29ec72eb2535af6554c68483a5f4856e3644cc1 Mon Sep 17 00:00:00 2001 From: Raul Jordan Date: Wed, 31 Aug 2022 20:25:36 -0400 Subject: [PATCH 12/15] fix up --- beacon-chain/sync/metrics.go | 16 ---------------- beacon-chain/sync/validate_aggregate_proof.go | 2 -- beacon-chain/sync/validate_beacon_attestation.go | 3 --- 3 files changed, 21 deletions(-) diff --git a/beacon-chain/sync/metrics.go b/beacon-chain/sync/metrics.go index 7922d03a2e62..6f235d5c638e 100644 --- a/beacon-chain/sync/metrics.go +++ b/beacon-chain/sync/metrics.go @@ -91,10 +91,6 @@ var ( ) // Attestation processing granular error tracking. - attWrongTargetEpochCount = promauto.NewCounter(prometheus.CounterOpts{ - Name: "gossip_attestation_wrong_target_total", - Help: "Increased when a gossip attestation fails target check", - }) attBadBlockCount = promauto.NewCounter(prometheus.CounterOpts{ Name: "gossip_attestation_bad_block_total", Help: "Increased when a gossip attestation references a bad block", @@ -103,22 +99,10 @@ var ( Name: "gossip_attestation_bad_lmd_consistency_total", Help: "Increased when a gossip attestation has bad LMD GHOST consistency", }) - attValidatorNotInCommitteeCount = promauto.NewCounter(prometheus.CounterOpts{ - Name: "gossip_attestation_validator_not_in_committee_total", - Help: "Increased when a gossip attestation's validator is not the right committee", - }) attBadSelectionProofCount = promauto.NewCounter(prometheus.CounterOpts{ Name: "gossip_attestation_bad_selection_proof_total", Help: "Increased when a gossip attestation has a bad selection proof", }) - attBadBitfieldLengthCount = promauto.NewCounter(prometheus.CounterOpts{ - Name: "gossip_attestation_bad_bitfield_length_total", - Help: "Increased when a gossip attestation has a bad bitfield length", - }) - attInvalidBitfieldCount = promauto.NewCounter(prometheus.CounterOpts{ - Name: "gossip_attestation_invalid_bitfield_total", - Help: "Increased when a gossip attestation has an invalid bitfield", - }) attBadSignatureBatchCount = promauto.NewCounter(prometheus.CounterOpts{ Name: "gossip_attestation_bad_signature_batch_total", Help: "Increased when a gossip attestation has a bad signature batch", diff --git a/beacon-chain/sync/validate_aggregate_proof.go b/beacon-chain/sync/validate_aggregate_proof.go index 128e2aef4461..8a5183844a7b 100644 --- a/beacon-chain/sync/validate_aggregate_proof.go +++ b/beacon-chain/sync/validate_aggregate_proof.go @@ -72,7 +72,6 @@ func (s *Service) validateAggregateAndProof(ctx context.Context, pid peer.ID, ms }) if err := helpers.ValidateSlotTargetEpoch(m.Message.Aggregate.Data); err != nil { - attWrongTargetEpochCount.Inc() return pubsub.ValidationReject, err } @@ -170,7 +169,6 @@ func (s *Service) validateAggregatedAtt(ctx context.Context, signed *ethpb.Signe if err := validateIndexInCommittee(ctx, bs, signed.Message.Aggregate, signed.Message.AggregatorIndex); err != nil { wrappedErr := errors.Wrapf(err, "Could not validate index in committee") tracing.AnnotateError(span, wrappedErr) - attValidatorNotInCommitteeCount.Inc() return pubsub.ValidationReject, wrappedErr } diff --git a/beacon-chain/sync/validate_beacon_attestation.go b/beacon-chain/sync/validate_beacon_attestation.go index db0caf2d1730..22d021cc63c3 100644 --- a/beacon-chain/sync/validate_beacon_attestation.go +++ b/beacon-chain/sync/validate_beacon_attestation.go @@ -85,7 +85,6 @@ func (s *Service) validateCommitteeIndexBeaconAttestation(ctx context.Context, p return pubsub.ValidationIgnore, err } if err := helpers.ValidateSlotTargetEpoch(att.Data); err != nil { - attWrongTargetEpochCount.Inc() return pubsub.ValidationReject, err } @@ -216,7 +215,6 @@ func (s *Service) validateUnaggregatedAttWithState(ctx context.Context, a *eth.A // Verify number of aggregation bits matches the committee size. if err := helpers.VerifyBitfieldLength(a.AggregationBits, uint64(len(committee))); err != nil { - attBadBitfieldLengthCount.Inc() return pubsub.ValidationReject, err } @@ -224,7 +222,6 @@ func (s *Service) validateUnaggregatedAttWithState(ctx context.Context, a *eth.A // Note: The Ethereum Beacon chain spec suggests (len(get_attesting_indices(state, attestation.data, attestation.aggregation_bits)) == 1) // however this validation can be achieved without use of get_attesting_indices which is an O(n) lookup. if a.AggregationBits.Count() != 1 || a.AggregationBits.BitIndices()[0] >= len(committee) { - attInvalidBitfieldCount.Inc() return pubsub.ValidationReject, errors.New("attestation bitfield is invalid") } From 0147b6382e57c12be8754eba5e70c543313c617c Mon Sep 17 00:00:00 2001 From: Raul Jordan Date: Wed, 31 Aug 2022 20:58:09 -0400 Subject: [PATCH 13/15] rev --- beacon-chain/sync/subscriber_beacon_aggregate_proof.go | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/beacon-chain/sync/subscriber_beacon_aggregate_proof.go b/beacon-chain/sync/subscriber_beacon_aggregate_proof.go index 63de7f7881df..6b5c434ac99d 100644 --- a/beacon-chain/sync/subscriber_beacon_aggregate_proof.go +++ b/beacon-chain/sync/subscriber_beacon_aggregate_proof.go @@ -24,14 +24,8 @@ func (s *Service) beaconAggregateProofSubscriber(_ context.Context, msg proto.Me // 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 := s.cfg.attPool.SaveUnaggregatedAttestation(a.Message.Aggregate); err != nil { - return err - } - return nil + return s.cfg.attPool.SaveUnaggregatedAttestation(a.Message.Aggregate) } - if err := s.cfg.attPool.SaveAggregatedAttestation(a.Message.Aggregate); err != nil { - return err - } - return nil + return s.cfg.attPool.SaveAggregatedAttestation(a.Message.Aggregate) } From 65a003448a78ae0b9b95021be24c89f969a4d1fe Mon Sep 17 00:00:00 2001 From: Raul Jordan Date: Wed, 31 Aug 2022 21:00:18 -0400 Subject: [PATCH 14/15] fix --- beacon-chain/sync/subscriber_beacon_attestation.go | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/beacon-chain/sync/subscriber_beacon_attestation.go b/beacon-chain/sync/subscriber_beacon_attestation.go index 341a54536144..0bc2dcc15cfa 100644 --- a/beacon-chain/sync/subscriber_beacon_attestation.go +++ b/beacon-chain/sync/subscriber_beacon_attestation.go @@ -33,10 +33,7 @@ func (s *Service) committeeIndexBeaconAttestationSubscriber(_ context.Context, m return nil } - if err := s.cfg.attPool.SaveUnaggregatedAttestation(a); err != nil { - return err - } - return nil + return s.cfg.attPool.SaveUnaggregatedAttestation(a) } func (_ *Service) persistentSubnetIndices() []uint64 { From fc5539bc3f4e0ede5e2167199ec8e39bb42c3506 Mon Sep 17 00:00:00 2001 From: Raul Jordan Date: Wed, 31 Aug 2022 21:05:03 -0400 Subject: [PATCH 15/15] rem --- beacon-chain/sync/metrics.go | 6 ------ beacon-chain/sync/validate_beacon_attestation.go | 4 ---- 2 files changed, 10 deletions(-) diff --git a/beacon-chain/sync/metrics.go b/beacon-chain/sync/metrics.go index 6f235d5c638e..91833c956abe 100644 --- a/beacon-chain/sync/metrics.go +++ b/beacon-chain/sync/metrics.go @@ -109,12 +109,6 @@ var ( }) // Attestation and block gossip verification performance. - unaggregatedAttestationVerificationGossipSummary = promauto.NewSummary( - prometheus.SummaryOpts{ - Name: "gossip_unaggregate_attestation_verification_milliseconds", - Help: "Time to verify gossiped, unaggregated attestations", - }, - ) aggregateAttestationVerificationGossipSummary = promauto.NewSummary( prometheus.SummaryOpts{ Name: "gossip_aggregate_attestation_verification_milliseconds", diff --git a/beacon-chain/sync/validate_beacon_attestation.go b/beacon-chain/sync/validate_beacon_attestation.go index 22d021cc63c3..8365c813a791 100644 --- a/beacon-chain/sync/validate_beacon_attestation.go +++ b/beacon-chain/sync/validate_beacon_attestation.go @@ -21,7 +21,6 @@ import ( "github.com/prysmaticlabs/prysm/v3/monitoring/tracing" eth "github.com/prysmaticlabs/prysm/v3/proto/prysm/v1alpha1" "github.com/prysmaticlabs/prysm/v3/proto/prysm/v1alpha1/attestation" - prysmTime "github.com/prysmaticlabs/prysm/v3/time" "github.com/prysmaticlabs/prysm/v3/time/slots" "go.opencensus.io/trace" ) @@ -33,7 +32,6 @@ import ( // - attestation.data.slot is within the last ATTESTATION_PROPAGATION_SLOT_RANGE slots (attestation.data.slot + ATTESTATION_PROPAGATION_SLOT_RANGE >= current_slot >= attestation.data.slot). // - The signature of attestation is valid. func (s *Service) validateCommitteeIndexBeaconAttestation(ctx context.Context, pid peer.ID, msg *pubsub.Message) (pubsub.ValidationResult, error) { - receivedTime := prysmTime.Now() if pid == s.cfg.p2p.PeerID() { return pubsub.ValidationAccept, nil } @@ -168,8 +166,6 @@ func (s *Service) validateCommitteeIndexBeaconAttestation(ctx context.Context, p msg.ValidatorData = att - unaggregatedAttestationVerificationGossipSummary.Observe(float64(prysmTime.Since(receivedTime).Milliseconds())) - return pubsub.ValidationAccept, nil }