Skip to content

Commit

Permalink
Remove validator count log (#14600)
Browse files Browse the repository at this point in the history
* remove validator count API call and update logs

* fixing test

* changelog

* removing unused function

* gaz

* casing

* fixing more tests
  • Loading branch information
james-prysm authored Nov 1, 2024
1 parent e6ffc07 commit f2ade3c
Show file tree
Hide file tree
Showing 8 changed files with 14 additions and 124 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ The format is based on Keep a Changelog, and this project adheres to Semantic Ve
### Removed

- Removed finalized validator index cache, no longer needed.
- Removed validator queue position log on key reload and wait for activation.

### Fixed

Expand Down
1 change: 0 additions & 1 deletion validator/client/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@ go_library(
"//consensus-types/blocks:go_default_library",
"//consensus-types/interfaces:go_default_library",
"//consensus-types/primitives:go_default_library",
"//consensus-types/validator:go_default_library",
"//crypto/bls:go_default_library",
"//crypto/hash:go_default_library",
"//crypto/rand:go_default_library",
Expand Down
7 changes: 1 addition & 6 deletions validator/client/key_reload.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,5 @@ func (v *validator) HandleKeyReload(ctx context.Context, currentKeys [][fieldpar
return false, err
}

valCount, err := v.getValidatorCount(ctx)
if err != nil {
return false, err
}

return v.checkAndLogValidatorStatus(valCount), nil
return v.checkAndLogValidatorStatus(), nil
}
12 changes: 0 additions & 12 deletions validator/client/key_reload_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,10 @@ import (

"github.com/pkg/errors"
fieldparams "github.com/prysmaticlabs/prysm/v5/config/fieldparams"
validator2 "github.com/prysmaticlabs/prysm/v5/consensus-types/validator"
ethpb "github.com/prysmaticlabs/prysm/v5/proto/prysm/v1alpha1"
"github.com/prysmaticlabs/prysm/v5/testing/assert"
"github.com/prysmaticlabs/prysm/v5/testing/require"
validatormock "github.com/prysmaticlabs/prysm/v5/testing/validator-mock"
"github.com/prysmaticlabs/prysm/v5/validator/client/iface"
"github.com/prysmaticlabs/prysm/v5/validator/client/testutil"
logTest "github.com/sirupsen/logrus/hooks/test"
"go.uber.org/mock/gomock"
Expand Down Expand Up @@ -48,11 +46,6 @@ func TestValidator_HandleKeyReload(t *testing.T) {
PublicKeys: [][]byte{inactive.pub[:], active.pub[:]},
},
).Return(resp, nil)
prysmChainClient.EXPECT().ValidatorCount(
gomock.Any(),
"head",
[]validator2.Status{validator2.Active},
).Return([]iface.ValidatorCount{}, nil)

anyActive, err := v.HandleKeyReload(context.Background(), [][fieldparams.BLSPubkeyLength]byte{inactive.pub, active.pub})
require.NoError(t, err)
Expand Down Expand Up @@ -85,11 +78,6 @@ func TestValidator_HandleKeyReload(t *testing.T) {
PublicKeys: [][]byte{kp.pub[:]},
},
).Return(resp, nil)
prysmChainClient.EXPECT().ValidatorCount(
gomock.Any(),
"head",
[]validator2.Status{validator2.Active},
).Return([]iface.ValidatorCount{}, nil)

anyActive, err := v.HandleKeyReload(context.Background(), [][fieldparams.BLSPubkeyLength]byte{kp.pub})
require.NoError(t, err)
Expand Down
33 changes: 7 additions & 26 deletions validator/client/validator.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import (
"encoding/json"
"fmt"
"io"
"math"
"strconv"
"strings"
"sync"
Expand Down Expand Up @@ -349,9 +348,9 @@ func (v *validator) WaitForSync(ctx context.Context) error {
}
}

func (v *validator) checkAndLogValidatorStatus(activeValCount int64) bool {
func (v *validator) checkAndLogValidatorStatus() bool {
nonexistentIndex := primitives.ValidatorIndex(^uint64(0))
var validatorActivated bool
var someAreActive bool
for _, s := range v.pubkeyToStatus {
fields := logrus.Fields{
"pubkey": fmt.Sprintf("%#x", bytesutil.Trunc(s.publicKey)),
Expand All @@ -369,29 +368,11 @@ func (v *validator) checkAndLogValidatorStatus(activeValCount int64) bool {
case ethpb.ValidatorStatus_UNKNOWN_STATUS:
log.Info("Waiting for deposit to be observed by beacon node")
case ethpb.ValidatorStatus_DEPOSITED:
if s.status.PositionInActivationQueue != 0 {
log.WithField(
"positionInActivationQueue", s.status.PositionInActivationQueue,
).Info("Deposit processed, entering activation queue after finalization")
}
log.Info("Validator deposited, entering activation queue after finalization")
case ethpb.ValidatorStatus_PENDING:
if activeValCount >= 0 && s.status.ActivationEpoch == params.BeaconConfig().FarFutureEpoch {
activationsPerEpoch :=
uint64(math.Max(float64(params.BeaconConfig().MinPerEpochChurnLimit), float64(uint64(activeValCount)/params.BeaconConfig().ChurnLimitQuotient)))
secondsPerEpoch := uint64(params.BeaconConfig().SlotsPerEpoch.Mul(params.BeaconConfig().SecondsPerSlot))
expectedWaitingTime :=
time.Duration((s.status.PositionInActivationQueue+activationsPerEpoch)/activationsPerEpoch*secondsPerEpoch) * time.Second
log.WithFields(logrus.Fields{
"positionInActivationQueue": s.status.PositionInActivationQueue,
"expectedWaitingTime": expectedWaitingTime.String(),
}).Info("Waiting to be assigned activation epoch")
} else if s.status.ActivationEpoch != params.BeaconConfig().FarFutureEpoch {
log.WithFields(logrus.Fields{
"activationEpoch": s.status.ActivationEpoch,
}).Info("Waiting for activation")
}
log.Info("Waiting for activation... Check validator queue status in a block explorer")
case ethpb.ValidatorStatus_ACTIVE, ethpb.ValidatorStatus_EXITING:
validatorActivated = true
someAreActive = true
log.WithFields(logrus.Fields{
"index": s.index,
}).Info("Validator activated")
Expand All @@ -401,11 +382,11 @@ func (v *validator) checkAndLogValidatorStatus(activeValCount int64) bool {
log.Warn("Invalid Eth1 deposit")
default:
log.WithFields(logrus.Fields{
"activationEpoch": s.status.ActivationEpoch,
"status": s.status.Status.String(),
}).Info("Validator status")
}
}
return validatorActivated
return someAreActive
}

// CanonicalHeadSlot returns the slot of canonical block currently found in the
Expand Down
20 changes: 3 additions & 17 deletions validator/client/validator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -806,7 +806,7 @@ func TestCheckAndLogValidatorStatus_OK(t *testing.T) {
PositionInActivationQueue: 30,
},
},
log: "Deposit processed, entering activation queue after finalization\" positionInActivationQueue=30 prefix=client pubkey=0x000000000000 status=DEPOSITED validatorIndex=30",
log: "Validator deposited, entering activation queue after finalization\" prefix=client pubkey=0x000000000000 status=DEPOSITED validatorIndex=30",
active: false,
},
{
Expand All @@ -820,21 +820,7 @@ func TestCheckAndLogValidatorStatus_OK(t *testing.T) {
PositionInActivationQueue: 6,
},
},
log: "Waiting to be assigned activation epoch\" expectedWaitingTime=12m48s positionInActivationQueue=6 prefix=client pubkey=0x000000000000 status=PENDING validatorIndex=50",
active: false,
},
{
name: "PENDING",
status: &validatorStatus{
publicKey: pubKeys[0],
index: 89,
status: &ethpb.ValidatorStatusResponse{
Status: ethpb.ValidatorStatus_PENDING,
ActivationEpoch: 60,
PositionInActivationQueue: 5,
},
},
log: "Waiting for activation\" activationEpoch=60 prefix=client pubkey=0x000000000000 status=PENDING validatorIndex=89",
log: "Waiting for activation... Check validator queue status in a block explorer\" prefix=client pubkey=0x000000000000 status=PENDING validatorIndex=50",
active: false,
},
{
Expand Down Expand Up @@ -889,7 +875,7 @@ func TestCheckAndLogValidatorStatus_OK(t *testing.T) {
pubkeyToStatus: make(map[[48]byte]*validatorStatus),
}
v.pubkeyToStatus[bytesutil.ToBytes48(test.status.publicKey)] = test.status
active := v.checkAndLogValidatorStatus(100)
active := v.checkAndLogValidatorStatus()
require.Equal(t, test.active, active)
if test.log != "" {
require.LogsContain(t, hook, test.log)
Expand Down
28 changes: 2 additions & 26 deletions validator/client/wait_for_activation.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,10 @@ import (

"github.com/pkg/errors"
fieldparams "github.com/prysmaticlabs/prysm/v5/config/fieldparams"
validator2 "github.com/prysmaticlabs/prysm/v5/consensus-types/validator"
"github.com/prysmaticlabs/prysm/v5/math"
"github.com/prysmaticlabs/prysm/v5/monitoring/tracing"
"github.com/prysmaticlabs/prysm/v5/monitoring/tracing/trace"
"github.com/prysmaticlabs/prysm/v5/time/slots"
"github.com/prysmaticlabs/prysm/v5/validator/client/iface"
octrace "go.opentelemetry.io/otel/trace"
)

Expand Down Expand Up @@ -61,14 +59,8 @@ func (v *validator) internalWaitForActivation(ctx context.Context, accountsChang
return v.retryWaitForActivation(ctx, span, err, "Connection broken while waiting for activation. Reconnecting...", accountsChangedChan)
}

// Step 4: Fetch validator count.
valCount, err := v.getValidatorCount(ctx)
if err != nil {
return err
}

// Step 5: Check and log validator statuses.
someAreActive := v.checkAndLogValidatorStatus(valCount)
// Step 4: Check and log validator statuses.
someAreActive := v.checkAndLogValidatorStatus()
if !someAreActive {
// Step 6: If no active validators, wait for accounts change, context cancellation, or next epoch.
select {
Expand All @@ -88,22 +80,6 @@ func (v *validator) internalWaitForActivation(ctx context.Context, accountsChang
return nil
}

// getValidatorCount is an api call to get the current validator count.
// "-1" indicates that validator count endpoint is not supported by the beacon node.
func (v *validator) getValidatorCount(ctx context.Context) (int64, error) {
// TODO: revisit https://github.com/prysmaticlabs/prysm/pull/12471#issuecomment-1568320970 to review if ValidatorCount api can be removed.

var valCount int64 = -1
valCounts, err := v.prysmChainClient.ValidatorCount(ctx, "head", []validator2.Status{validator2.Active})
if err != nil && !errors.Is(err, iface.ErrNotSupported) {
return -1, errors.Wrap(err, "could not get active validator count")
}
if len(valCounts) > 0 {
valCount = int64(valCounts[0].Count)
}
return valCount, nil
}

func (v *validator) retryWaitForActivation(ctx context.Context, span octrace.Span, err error, message string, accountsChangedChan <-chan [][fieldparams.BLSPubkeyLength]byte) error {
tracing.AnnotateError(span, err)
attempts := activationAttempts(ctx)
Expand Down
36 changes: 0 additions & 36 deletions validator/client/wait_for_activation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ import (
"github.com/prysmaticlabs/prysm/v5/testing/require"
validatormock "github.com/prysmaticlabs/prysm/v5/testing/validator-mock"
walletMock "github.com/prysmaticlabs/prysm/v5/validator/accounts/testing"
"github.com/prysmaticlabs/prysm/v5/validator/client/iface"
"github.com/prysmaticlabs/prysm/v5/validator/client/testutil"
"github.com/prysmaticlabs/prysm/v5/validator/keymanager/derived"
constant "github.com/prysmaticlabs/prysm/v5/validator/testing"
Expand Down Expand Up @@ -46,16 +45,6 @@ func TestWaitActivation_Exiting_OK(t *testing.T) {
PublicKeys: [][]byte{kp.pub[:]},
},
).Return(resp, nil)
prysmChainClient.EXPECT().ValidatorCount(
gomock.Any(),
"head",
gomock.Any(),
).Return([]iface.ValidatorCount{
{
Status: "EXITING",
Count: 1,
},
}, nil).AnyTimes()

require.NoError(t, v.WaitForActivation(ctx, nil))
require.Equal(t, 1, len(v.pubkeyToStatus))
Expand Down Expand Up @@ -93,16 +82,6 @@ func TestWaitForActivation_RefetchKeys(t *testing.T) {
PublicKeys: [][]byte{kp.pub[:]},
},
).Return(resp, nil)
prysmChainClient.EXPECT().ValidatorCount(
gomock.Any(),
"head",
gomock.Any(),
).Return([]iface.ValidatorCount{
{
Status: "ACTIVE",
Count: 1,
},
}, nil)

accountChan := make(chan [][fieldparams.BLSPubkeyLength]byte)
sub := km.SubscribeAccountChanges(accountChan)
Expand Down Expand Up @@ -163,11 +142,6 @@ func TestWaitForActivation_AccountsChanged(t *testing.T) {
},
).Return(activeResp, nil))

prysmChainClient.EXPECT().ValidatorCount(
gomock.Any(),
"head",
gomock.Any(),
).Return([]iface.ValidatorCount{}, nil).AnyTimes()
chainClient.EXPECT().ChainHead(
gomock.Any(),
gomock.Any(),
Expand Down Expand Up @@ -246,11 +220,6 @@ func TestWaitForActivation_AccountsChanged(t *testing.T) {
},
).Return(activeResp, nil))

prysmChainClient.EXPECT().ValidatorCount(
gomock.Any(),
"head",
gomock.Any(),
).Return([]iface.ValidatorCount{}, nil).AnyTimes()
chainClient.EXPECT().ChainHead(
gomock.Any(),
gomock.Any(),
Expand Down Expand Up @@ -295,11 +264,6 @@ func TestWaitForActivation_AttemptsReconnectionOnFailure(t *testing.T) {
gomock.Any(),
gomock.Any(),
).Return(activeResp, nil))
prysmChainClient.EXPECT().ValidatorCount(
gomock.Any(),
"head",
gomock.Any(),
).Return([]iface.ValidatorCount{}, nil).AnyTimes()
chainClient.EXPECT().ChainHead(
gomock.Any(),
gomock.Any(),
Expand Down

0 comments on commit f2ade3c

Please sign in to comment.