Skip to content

Commit

Permalink
undoing some reverts, validator count api should not be called and sh…
Browse files Browse the repository at this point in the history
…ould remain removed
  • Loading branch information
james-prysm committed Nov 22, 2024
1 parent 13417ac commit ff5df53
Show file tree
Hide file tree
Showing 6 changed files with 12 additions and 131 deletions.
16 changes: 1 addition & 15 deletions validator/client/key_reload.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,8 @@ package client
import (
"context"

"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/monitoring/tracing/trace"
"github.com/prysmaticlabs/prysm/v5/validator/client/iface"
)

// HandleKeyReload makes sure the validator keeps operating correctly after a change to the underlying keys.
Expand All @@ -20,16 +17,5 @@ func (v *validator) HandleKeyReload(ctx context.Context, currentKeys [][fieldpar
return false, err
}

// "-1" indicates that validator count endpoint is not supported by the beacon node.
var valCount int64 = -1
valCounts, err := v.prysmChainClient.ValidatorCount(ctx, "head", []validator2.Status{validator2.Active})
if err != nil && !errors.Is(err, iface.ErrNotSupported) {
return false, errors.Wrap(err, "could not get active validator count")
}

if len(valCounts) > 0 {
valCount = int64(valCounts[0].Count)
}

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
25 changes: 3 additions & 22 deletions validator/client/validator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -372,11 +372,6 @@ func TestWaitMultipleActivation_LogsActivationEpochOK(t *testing.T) {
resp,
nil,
)
prysmChainClient.EXPECT().ValidatorCount(
gomock.Any(),
"head",
[]validatorType.Status{validatorType.Active},
).Return([]iface.ValidatorCount{}, nil)
require.NoError(t, v.WaitForActivation(ctx, nil), "Could not wait for activation")
require.LogsContain(t, hook, "Validator activated")
}
Expand Down Expand Up @@ -860,7 +855,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 @@ -874,21 +869,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 @@ -943,7 +924,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
15 changes: 1 addition & 14 deletions validator/client/wait_for_activation.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,11 @@ 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/encoding/bytesutil"
"github.com/prysmaticlabs/prysm/v5/math"
"github.com/prysmaticlabs/prysm/v5/monitoring/tracing"
"github.com/prysmaticlabs/prysm/v5/monitoring/tracing/trace"
ethpb "github.com/prysmaticlabs/prysm/v5/proto/prysm/v1alpha1"
"github.com/prysmaticlabs/prysm/v5/validator/client/iface"
)

// WaitForActivation checks whether the validator pubkey is in the active
Expand Down Expand Up @@ -117,18 +115,7 @@ func (v *validator) internalWaitForActivation(ctx context.Context, accountsChang
}
}

// "-1" indicates that validator count endpoint is not supported by the beacon node.
var valCount int64 = -1
valCounts, err := v.prysmChainClient.ValidatorCount(ctx, "head", []validator2.Status{validator2.Active})
if err != nil && !errors.Is(err, iface.ErrNotSupported) {
return errors.Wrap(err, "could not get active validator count")
}

if len(valCounts) > 0 {
valCount = int64(valCounts[0].Count)
}

someAreActive = v.checkAndLogValidatorStatus(valCount)
someAreActive = v.checkAndLogValidatorStatus()
}
}

Expand Down
42 changes: 0 additions & 42 deletions validator/client/wait_for_activation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,12 @@ import (
"github.com/pkg/errors"
fieldparams "github.com/prysmaticlabs/prysm/v5/config/fieldparams"
"github.com/prysmaticlabs/prysm/v5/config/params"
validatorType "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/mock"
"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/keymanager/derived"
constant "github.com/prysmaticlabs/prysm/v5/validator/testing"
logTest "github.com/sirupsen/logrus/hooks/test"
Expand Down Expand Up @@ -75,11 +73,6 @@ func TestWaitActivation_StreamSetupFails_AttemptsToReconnect(t *testing.T) {
PublicKeys: [][]byte{kp.pub[:]},
},
).Return(clientStream, errors.New("failed stream")).Return(clientStream, nil)
prysmChainClient.EXPECT().ValidatorCount(
gomock.Any(),
"head",
[]validatorType.Status{validatorType.Active},
).Return([]iface.ValidatorCount{}, nil)
resp := generateMockStatusResponse([][]byte{kp.pub[:]})
resp.Statuses[0].Status.Status = ethpb.ValidatorStatus_ACTIVE
clientStream.EXPECT().Recv().Return(resp, nil)
Expand Down Expand Up @@ -107,11 +100,6 @@ func TestWaitForActivation_ReceiveErrorFromStream_AttemptsReconnection(t *testin
PublicKeys: [][]byte{kp.pub[:]},
},
).Return(clientStream, nil)
prysmChainClient.EXPECT().ValidatorCount(
gomock.Any(),
"head",
[]validatorType.Status{validatorType.Active},
).Return([]iface.ValidatorCount{}, nil)
// A stream fails the first time, but succeeds the second time.
resp := generateMockStatusResponse([][]byte{kp.pub[:]})
resp.Statuses[0].Status.Status = ethpb.ValidatorStatus_ACTIVE
Expand Down Expand Up @@ -147,11 +135,6 @@ func TestWaitActivation_LogsActivationEpochOK(t *testing.T) {
PublicKeys: [][]byte{kp.pub[:]},
},
).Return(clientStream, nil)
prysmChainClient.EXPECT().ValidatorCount(
gomock.Any(),
"head",
[]validatorType.Status{validatorType.Active},
).Return([]iface.ValidatorCount{}, nil)
clientStream.EXPECT().Recv().Return(
resp,
nil,
Expand Down Expand Up @@ -183,11 +166,6 @@ func TestWaitForActivation_Exiting(t *testing.T) {
PublicKeys: [][]byte{kp.pub[:]},
},
).Return(clientStream, nil)
prysmChainClient.EXPECT().ValidatorCount(
gomock.Any(),
"head",
[]validatorType.Status{validatorType.Active},
).Return([]iface.ValidatorCount{}, nil)
clientStream.EXPECT().Recv().Return(
resp,
nil,
Expand Down Expand Up @@ -227,11 +205,6 @@ func TestWaitForActivation_RefetchKeys(t *testing.T) {
PublicKeys: [][]byte{kp.pub[:]},
},
).Return(clientStream, nil)
prysmChainClient.EXPECT().ValidatorCount(
gomock.Any(),
"head",
[]validatorType.Status{validatorType.Active},
).Return([]iface.ValidatorCount{}, nil)
clientStream.EXPECT().Recv().Return(
resp,
nil)
Expand Down Expand Up @@ -285,11 +258,6 @@ func TestWaitForActivation_AccountsChanged(t *testing.T) {
time.Sleep(time.Second * 2)
return inactiveClientStream, nil
})
prysmChainClient.EXPECT().ValidatorCount(
gomock.Any(),
"head",
[]validatorType.Status{validatorType.Active},
).Return([]iface.ValidatorCount{}, nil).AnyTimes()
inactiveClientStream.EXPECT().Recv().Return(
inactiveResp,
nil,
Expand Down Expand Up @@ -378,11 +346,6 @@ func TestWaitForActivation_AccountsChanged(t *testing.T) {
time.Sleep(time.Second * 2)
return inactiveClientStream, nil
})
prysmChainClient.EXPECT().ValidatorCount(
gomock.Any(),
"head",
[]validatorType.Status{validatorType.Active},
).Return([]iface.ValidatorCount{}, nil).AnyTimes()
inactiveClientStream.EXPECT().Recv().Return(
inactiveResp,
nil,
Expand Down Expand Up @@ -440,11 +403,6 @@ func TestWaitActivation_NotAllValidatorsActivatedOK(t *testing.T) {
gomock.Any(),
gomock.Any(),
).Return(clientStream, nil)
prysmChainClient.EXPECT().ValidatorCount(
gomock.Any(),
"head",
[]validatorType.Status{validatorType.Active},
).Return([]iface.ValidatorCount{}, nil).Times(2)
clientStream.EXPECT().Recv().Return(
&ethpb.ValidatorActivationResponse{},
nil,
Expand Down

0 comments on commit ff5df53

Please sign in to comment.