Skip to content

Commit

Permalink
Merge branch 'develop' into update-geth-to-v1.14
Browse files Browse the repository at this point in the history
  • Loading branch information
james-prysm authored Nov 1, 2024
2 parents e80f522 + a265cf0 commit cde5d63
Show file tree
Hide file tree
Showing 10 changed files with 48 additions and 128 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,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
8 changes: 4 additions & 4 deletions proto/engine/v1/electra.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,16 +28,16 @@ const (

func (ebe *ExecutionBundleElectra) GetDecodedExecutionRequests() (*ExecutionRequests, error) {
requests := &ExecutionRequests{}
var prevTypeNum uint8
var prevTypeNum *uint8
for i := range ebe.ExecutionRequests {
requestType, requestListInSSZBytes, err := decodeExecutionRequest(ebe.ExecutionRequests[i])
if err != nil {
return nil, err
}
if prevTypeNum > requestType {
return nil, errors.New("invalid execution request type order, requests should be in sorted order")
if prevTypeNum != nil && *prevTypeNum >= requestType {
return nil, errors.New("invalid execution request type order or duplicate requests, requests should be in sorted order and unique")
}
prevTypeNum = requestType
prevTypeNum = &requestType
switch requestType {
case DepositRequestType:
drs, err := unmarshalDeposits(requestListInSSZBytes)
Expand Down
30 changes: 30 additions & 0 deletions proto/engine/v1/electra_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,36 @@ func TestGetDecodedExecutionRequests(t *testing.T) {
_, err = ebe.GetDecodedExecutionRequests()
require.ErrorContains(t, "invalid execution request, length less than 1", err)
})
t.Run("a duplicate request should fail", func(t *testing.T) {
withdrawalRequestBytes, err := hexutil.Decode("0x6400000000000000000000000000000000000000" +
"6500000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000040597307000000")
require.NoError(t, err)
withdrawalRequestBytes2, err := hexutil.Decode("0x6400000000000000000000000000000000000000" +
"6500000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000040597307000000")
require.NoError(t, err)
ebe := &enginev1.ExecutionBundleElectra{
ExecutionRequests: [][]byte{append([]byte{uint8(enginev1.WithdrawalRequestType)}, withdrawalRequestBytes...), append([]byte{uint8(enginev1.WithdrawalRequestType)}, withdrawalRequestBytes2...)},
}
_, err = ebe.GetDecodedExecutionRequests()
require.ErrorContains(t, "requests should be in sorted order and unique", err)
})
t.Run("a duplicate withdrawals ( non 0 request type )request should fail", func(t *testing.T) {
depositRequestBytes, err := hexutil.Decode("0x610000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000" +
"620000000000000000000000000000000000000000000000000000000000000000" +
"4059730700000063000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000" +
"00000000000000000000000000000000000000000000000000000000000000000000000000000000")
require.NoError(t, err)
depositRequestBytes2, err := hexutil.Decode("0x610000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000" +
"620000000000000000000000000000000000000000000000000000000000000000" +
"4059730700000063000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000" +
"00000000000000000000000000000000000000000000000000000000000000000000000000000000")
require.NoError(t, err)
ebe := &enginev1.ExecutionBundleElectra{
ExecutionRequests: [][]byte{append([]byte{uint8(enginev1.DepositRequestType)}, depositRequestBytes...), append([]byte{uint8(enginev1.DepositRequestType)}, depositRequestBytes2...)},
}
_, err = ebe.GetDecodedExecutionRequests()
require.ErrorContains(t, "requests should be in sorted order and unique", err)
})
t.Run("If a request type is provided, but the request list is shorter than the ssz of 1 request we error", func(t *testing.T) {
consolidationRequestBytes, err := hexutil.Decode("0x6600000000000000000000000000000000000000" +
"670000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000" +
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 cde5d63

Please sign in to comment.