From a5ce6db38ebe74c6954adcd4cfcfae8ddcf51dbe Mon Sep 17 00:00:00 2001 From: terence tsao Date: Tue, 20 Oct 2020 12:14:24 -0700 Subject: [PATCH] Update participation metrics (#7582) * Update participation metrics * Add unhappy tests --- beacon-chain/blockchain/BUILD.bazel | 1 + beacon-chain/blockchain/metrics.go | 51 +++++++++++-------- beacon-chain/blockchain/metrics_test.go | 26 ++++++++++ beacon-chain/blockchain/process_block.go | 12 +++-- beacon-chain/blockchain/process_block_test.go | 12 +++++ .../core/epoch/precompute/attestation.go | 5 -- 6 files changed, 77 insertions(+), 30 deletions(-) create mode 100644 beacon-chain/blockchain/metrics_test.go diff --git a/beacon-chain/blockchain/BUILD.bazel b/beacon-chain/blockchain/BUILD.bazel index f942aed1d20..3ad22241557 100644 --- a/beacon-chain/blockchain/BUILD.bazel +++ b/beacon-chain/blockchain/BUILD.bazel @@ -83,6 +83,7 @@ go_test( "checkpoint_info_cache_test.go", "head_test.go", "info_test.go", + "metrics_test.go", "process_attestation_test.go", "process_block_test.go", "receive_attestation_test.go", diff --git a/beacon-chain/blockchain/metrics.go b/beacon-chain/blockchain/metrics.go index 4900e204da4..590790a3196 100644 --- a/beacon-chain/blockchain/metrics.go +++ b/beacon-chain/blockchain/metrics.go @@ -1,6 +1,8 @@ package blockchain import ( + "context" + "github.com/prometheus/client_golang/prometheus" "github.com/prometheus/client_golang/prometheus/promauto" ethpb "github.com/prysmaticlabs/ethereumapis/eth/v1alpha1" @@ -126,8 +128,8 @@ func reportSlotMetrics(stateSlot, headSlot, clockSlot uint64, finalizedCheckpoin } // reportEpochMetrics reports epoch related metrics. -func reportEpochMetrics(state *stateTrie.BeaconState) { - currentEpoch := state.Slot() / params.BeaconConfig().SlotsPerEpoch +func reportEpochMetrics(ctx context.Context, postState *stateTrie.BeaconState, headState *stateTrie.BeaconState) error { + currentEpoch := postState.Slot() / params.BeaconConfig().SlotsPerEpoch // Validator instances pendingInstances := 0 @@ -145,8 +147,8 @@ func reportEpochMetrics(state *stateTrie.BeaconState) { slashingBalance := uint64(0) slashingEffectiveBalance := uint64(0) - for i, validator := range state.Validators() { - bal, err := state.BalanceAtIndex(uint64(i)) + for i, validator := range postState.Validators() { + bal, err := postState.BalanceAtIndex(uint64(i)) if err != nil { log.Errorf("Could not load validator balance: %v", err) continue @@ -195,31 +197,40 @@ func reportEpochMetrics(state *stateTrie.BeaconState) { validatorsEffectiveBalance.WithLabelValues("Slashing").Set(float64(slashingEffectiveBalance)) // Last justified slot - beaconCurrentJustifiedEpoch.Set(float64(state.CurrentJustifiedCheckpoint().Epoch)) - beaconCurrentJustifiedRoot.Set(float64(bytesutil.ToLowInt64(state.CurrentJustifiedCheckpoint().Root))) + beaconCurrentJustifiedEpoch.Set(float64(postState.CurrentJustifiedCheckpoint().Epoch)) + beaconCurrentJustifiedRoot.Set(float64(bytesutil.ToLowInt64(postState.CurrentJustifiedCheckpoint().Root))) // Last previous justified slot - beaconPrevJustifiedEpoch.Set(float64(state.PreviousJustifiedCheckpoint().Epoch)) - beaconPrevJustifiedRoot.Set(float64(bytesutil.ToLowInt64(state.PreviousJustifiedCheckpoint().Root))) + beaconPrevJustifiedEpoch.Set(float64(postState.PreviousJustifiedCheckpoint().Epoch)) + beaconPrevJustifiedRoot.Set(float64(bytesutil.ToLowInt64(postState.PreviousJustifiedCheckpoint().Root))) // Last finalized slot - beaconFinalizedEpoch.Set(float64(state.FinalizedCheckpointEpoch())) - beaconFinalizedRoot.Set(float64(bytesutil.ToLowInt64(state.FinalizedCheckpoint().Root))) - - currentEth1DataDepositCount.Set(float64(state.Eth1Data().DepositCount)) + beaconFinalizedEpoch.Set(float64(postState.FinalizedCheckpointEpoch())) + beaconFinalizedRoot.Set(float64(bytesutil.ToLowInt64(postState.FinalizedCheckpoint().Root))) + currentEth1DataDepositCount.Set(float64(postState.Eth1Data().DepositCount)) - if precompute.Balances != nil { - totalEligibleBalances.Set(float64(precompute.Balances.ActivePrevEpoch)) - totalVotedTargetBalances.Set(float64(precompute.Balances.PrevEpochTargetAttested)) - prevEpochActiveBalances.Set(float64(precompute.Balances.ActivePrevEpoch)) - prevEpochSourceBalances.Set(float64(precompute.Balances.PrevEpochAttested)) - prevEpochTargetBalances.Set(float64(precompute.Balances.PrevEpochTargetAttested)) - prevEpochHeadBalances.Set(float64(precompute.Balances.PrevEpochHeadAttested)) + // Validator participation should be viewed on the canonical chain. + v, b, err := precompute.New(ctx, headState) + if err != nil { + return err + } + _, b, err = precompute.ProcessAttestations(ctx, headState, v, b) + if err != nil { + return err } - refMap := state.FieldReferencesCount() + totalEligibleBalances.Set(float64(b.ActivePrevEpoch)) + totalVotedTargetBalances.Set(float64(b.PrevEpochTargetAttested)) + prevEpochActiveBalances.Set(float64(b.ActivePrevEpoch)) + prevEpochSourceBalances.Set(float64(b.PrevEpochAttested)) + prevEpochTargetBalances.Set(float64(b.PrevEpochTargetAttested)) + prevEpochHeadBalances.Set(float64(b.PrevEpochHeadAttested)) + + refMap := postState.FieldReferencesCount() for name, val := range refMap { stateTrieReferences.WithLabelValues(name).Set(float64(val)) } + + return nil } func reportAttestationInclusion(blk *ethpb.BeaconBlock) { diff --git a/beacon-chain/blockchain/metrics_test.go b/beacon-chain/blockchain/metrics_test.go new file mode 100644 index 00000000000..fd653466c81 --- /dev/null +++ b/beacon-chain/blockchain/metrics_test.go @@ -0,0 +1,26 @@ +package blockchain + +import ( + "context" + "testing" + + pb "github.com/prysmaticlabs/prysm/proto/beacon/p2p/v1" + "github.com/prysmaticlabs/prysm/shared/testutil" + "github.com/prysmaticlabs/prysm/shared/testutil/require" +) + +func TestReportEpochMetrics_BadHeadState(t *testing.T) { + s := testutil.NewBeaconState() + h := testutil.NewBeaconState() + require.NoError(t, h.SetValidators(nil)) + err := reportEpochMetrics(context.Background(), s, h) + require.ErrorContains(t, "failed to initialize precompute: nil validators in state", err) +} + +func TestReportEpochMetrics_BadAttestation(t *testing.T) { + s := testutil.NewBeaconState() + h := testutil.NewBeaconState() + require.NoError(t, h.SetCurrentEpochAttestations([]*pb.PendingAttestation{{InclusionDelay: 0}})) + err := reportEpochMetrics(context.Background(), s, h) + require.ErrorContains(t, "attestation with inclusion delay of 0", err) +} diff --git a/beacon-chain/blockchain/process_block.go b/beacon-chain/blockchain/process_block.go index 3dd82f4cf50..1b8082c2bd1 100644 --- a/beacon-chain/blockchain/process_block.go +++ b/beacon-chain/blockchain/process_block.go @@ -140,7 +140,7 @@ func (s *Service) onBlock(ctx context.Context, signed *ethpb.SignedBeaconBlock, defer reportAttestationInclusion(b) - return s.handleEpochBoundary(postState) + return s.handleEpochBoundary(ctx, postState) } // onBlockInitialSyncStateTransition is called when an initial sync block is received. @@ -209,7 +209,7 @@ func (s *Service) onBlockInitialSyncStateTransition(ctx context.Context, signed } } - return s.handleEpochBoundary(postState) + return s.handleEpochBoundary(ctx, postState) } func (s *Service) onBlockBatch(ctx context.Context, blks []*ethpb.SignedBeaconBlock, @@ -254,7 +254,7 @@ func (s *Service) onBlockBatch(ctx context.Context, blks []*ethpb.SignedBeaconBl // Save potential boundary states. if helpers.IsEpochStart(preState.Slot()) { boundaries[blockRoots[i]] = preState.Copy() - if err := s.handleEpochBoundary(preState); err != nil { + if err := s.handleEpochBoundary(ctx, preState); err != nil { return nil, nil, errors.Wrap(err, "could not handle epoch boundary state") } } @@ -322,9 +322,11 @@ func (s *Service) handleBlockAfterBatchVerify(ctx context.Context, signed *ethpb } // Epoch boundary bookkeeping such as logging epoch summaries. -func (s *Service) handleEpochBoundary(postState *stateTrie.BeaconState) error { +func (s *Service) handleEpochBoundary(ctx context.Context, postState *stateTrie.BeaconState) error { if postState.Slot() >= s.nextEpochBoundarySlot { - reportEpochMetrics(postState) + if err := reportEpochMetrics(ctx, postState, s.head.state); err != nil { + return err + } var err error s.nextEpochBoundarySlot, err = helpers.StartSlot(helpers.NextEpoch(postState)) if err != nil { diff --git a/beacon-chain/blockchain/process_block_test.go b/beacon-chain/blockchain/process_block_test.go index 2d79a8bb299..9738e2d3f6c 100644 --- a/beacon-chain/blockchain/process_block_test.go +++ b/beacon-chain/blockchain/process_block_test.go @@ -837,3 +837,15 @@ func TestUpdateJustifiedInitSync(t *testing.T) { require.NoError(t, err) assert.DeepEqual(t, newCp, cp, "Incorrect current justified checkpoint in db") } + +func TestHandleEpochBoundary_BadMetrics(t *testing.T) { + ctx := context.Background() + cfg := &Config{} + service, err := NewService(ctx, cfg) + require.NoError(t, err) + + s := testutil.NewBeaconState() + require.NoError(t, s.SetSlot(1)) + service.head = &head{} + require.ErrorContains(t, "failed to initialize precompute: nil inner state", service.handleEpochBoundary(ctx, s)) +} diff --git a/beacon-chain/core/epoch/precompute/attestation.go b/beacon-chain/core/epoch/precompute/attestation.go index 25eda6787af..6a7fd9ba1fd 100644 --- a/beacon-chain/core/epoch/precompute/attestation.go +++ b/beacon-chain/core/epoch/precompute/attestation.go @@ -14,10 +14,6 @@ import ( "go.opencensus.io/trace" ) -// Balances stores balances such as prev/current total validator balances, attested balances and more. -// It's used for metrics reporting. -var Balances *Balance - // ProcessAttestations process the attestations in state and update individual validator's pre computes, // it also tracks and updates epoch attesting balances. func ProcessAttestations( @@ -56,7 +52,6 @@ func ProcessAttestations( } pBal = UpdateBalance(vp, pBal) - Balances = pBal return vp, pBal, nil }