diff --git a/beacon-chain/core/blocks/exit.go b/beacon-chain/core/blocks/exit.go index 046b9bbc1a49..99da22fae90a 100644 --- a/beacon-chain/core/blocks/exit.go +++ b/beacon-chain/core/blocks/exit.go @@ -98,6 +98,8 @@ func ProcessVoluntaryExits( // assert get_current_epoch(state) >= voluntary_exit.epoch // # Verify the validator has been active long enough // assert get_current_epoch(state) >= validator.activation_epoch + SHARD_COMMITTEE_PERIOD +// # Only exit validator if it has no pending withdrawals in the queue +// assert get_pending_balance_to_withdraw(state, voluntary_exit.validator_index) == 0 # [New in Electra:EIP7251] // # Verify signature // domain = get_domain(state, DOMAIN_VOLUNTARY_EXIT, voluntary_exit.epoch) // signing_root = compute_signing_root(voluntary_exit, domain) @@ -113,7 +115,6 @@ func VerifyExitAndSignature( return errors.New("nil exit") } - currentSlot := state.Slot() fork := state.Fork() genesisRoot := state.GenesisValidatorsRoot() @@ -128,7 +129,7 @@ func VerifyExitAndSignature( } exit := signed.Exit - if err := verifyExitConditions(validator, currentSlot, exit); err != nil { + if err := verifyExitConditions(state, validator, exit); err != nil { return err } domain, err := signing.Domain(fork, exit.Epoch, params.BeaconConfig().DomainVoluntaryExit, genesisRoot) @@ -157,14 +158,16 @@ func VerifyExitAndSignature( // assert get_current_epoch(state) >= voluntary_exit.epoch // # Verify the validator has been active long enough // assert get_current_epoch(state) >= validator.activation_epoch + SHARD_COMMITTEE_PERIOD +// # Only exit validator if it has no pending withdrawals in the queue +// assert get_pending_balance_to_withdraw(state, voluntary_exit.validator_index) == 0 # [New in Electra:EIP7251] // # Verify signature // domain = get_domain(state, DOMAIN_VOLUNTARY_EXIT, voluntary_exit.epoch) // signing_root = compute_signing_root(voluntary_exit, domain) // assert bls.Verify(validator.pubkey, signing_root, signed_voluntary_exit.signature) // # Initiate exit // initiate_validator_exit(state, voluntary_exit.validator_index) -func verifyExitConditions(validator state.ReadOnlyValidator, currentSlot primitives.Slot, exit *ethpb.VoluntaryExit) error { - currentEpoch := slots.ToEpoch(currentSlot) +func verifyExitConditions(st state.ReadOnlyBeaconState, validator state.ReadOnlyValidator, exit *ethpb.VoluntaryExit) error { + currentEpoch := slots.ToEpoch(st.Slot()) // Verify the validator is active. if !helpers.IsActiveValidatorUsingTrie(validator, currentEpoch) { return errors.New("non-active validator cannot exit") @@ -187,5 +190,17 @@ func verifyExitConditions(validator state.ReadOnlyValidator, currentSlot primiti validator.ActivationEpoch()+params.BeaconConfig().ShardCommitteePeriod, ) } + + if st.Version() >= version.Electra { + // Only exit validator if it has no pending withdrawals in the queue. + pbw, err := st.PendingBalanceToWithdraw(exit.ValidatorIndex) + if err != nil { + return fmt.Errorf("unable to retrieve pending balance to withdraw for validator %d: %w", exit.ValidatorIndex, err) + } + if pbw != 0 { + return fmt.Errorf("validator %d must have no pending balance to withdraw, got %d pending balance to withdraw", exit.ValidatorIndex, pbw) + } + } + return nil } diff --git a/beacon-chain/core/blocks/exit_test.go b/beacon-chain/core/blocks/exit_test.go index d40f35f7a2be..bda5f7ef81b4 100644 --- a/beacon-chain/core/blocks/exit_test.go +++ b/beacon-chain/core/blocks/exit_test.go @@ -135,6 +135,11 @@ func TestProcessVoluntaryExits_AppliesCorrectStatus(t *testing.T) { } func TestVerifyExitAndSignature(t *testing.T) { + // Remove after electra fork epoch is defined. + cfg := params.BeaconConfig() + cfg.ElectraForkEpoch = cfg.DenebForkEpoch * 2 + params.SetActiveTestCleanup(t, cfg) + // End remove section. denebSlot, err := slots.EpochStart(params.BeaconConfig().DenebForkEpoch) require.NoError(t, err) tests := []struct { @@ -167,7 +172,7 @@ func TestVerifyExitAndSignature(t *testing.T) { wantErr: "nil exit", }, { - name: "Happy Path", + name: "Happy Path phase0", setup: func() (*ethpb.Validator, *ethpb.SignedVoluntaryExit, state.ReadOnlyBeaconState, error) { fork := ðpb.Fork{ PreviousVersion: params.BeaconConfig().GenesisForkVersion, @@ -275,6 +280,52 @@ func TestVerifyExitAndSignature(t *testing.T) { return validator, signedExit, bs, nil }, }, + { + name: "EIP-7251 - pending balance to withdraw must be zero", + setup: func() (*ethpb.Validator, *ethpb.SignedVoluntaryExit, state.ReadOnlyBeaconState, error) { + fork := ðpb.Fork{ + PreviousVersion: params.BeaconConfig().DenebForkVersion, + CurrentVersion: params.BeaconConfig().ElectraForkVersion, + Epoch: params.BeaconConfig().ElectraForkEpoch, + } + signedExit := ðpb.SignedVoluntaryExit{ + Exit: ðpb.VoluntaryExit{ + Epoch: params.BeaconConfig().DenebForkEpoch + 1, + ValidatorIndex: 0, + }, + } + electraSlot, err := slots.EpochStart(params.BeaconConfig().ElectraForkEpoch) + require.NoError(t, err) + bs, keys := util.DeterministicGenesisState(t, 1) + bs, err = state_native.InitializeFromProtoUnsafeElectra(ðpb.BeaconStateElectra{ + GenesisValidatorsRoot: bs.GenesisValidatorsRoot(), + Fork: fork, + Slot: electraSlot, + Validators: bs.Validators(), + }) + if err != nil { + return nil, nil, nil, err + } + validator := bs.Validators()[0] + validator.ActivationEpoch = 1 + err = bs.UpdateValidatorAtIndex(0, validator) + require.NoError(t, err) + sb, err := signing.ComputeDomainAndSign(bs, signedExit.Exit.Epoch, signedExit.Exit, params.BeaconConfig().DomainVoluntaryExit, keys[0]) + require.NoError(t, err) + sig, err := bls.SignatureFromBytes(sb) + require.NoError(t, err) + signedExit.Signature = sig.Marshal() + + // Give validator a pending balance to withdraw. + require.NoError(t, bs.AppendPendingPartialWithdrawal(ðpb.PendingPartialWithdrawal{ + Index: 0, + Amount: 500, + })) + + return validator, signedExit, bs, nil + }, + wantErr: "validator 0 must have no pending balance to withdraw", + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { diff --git a/beacon-chain/core/electra/registry_updates.go b/beacon-chain/core/electra/registry_updates.go index f7991ea594d9..89824fb84fbe 100644 --- a/beacon-chain/core/electra/registry_updates.go +++ b/beacon-chain/core/electra/registry_updates.go @@ -58,7 +58,7 @@ func ProcessRegistryUpdates(ctx context.Context, st state.BeaconState) error { } // Collect validators eligible for activation and not yet dequeued for activation. - if helpers.IsEligibleForActivationUsingTrie(st, val) { + if helpers.IsEligibleForActivationUsingROVal(st, val) { eligibleForActivation = append(eligibleForActivation, primitives.ValidatorIndex(idx)) } diff --git a/beacon-chain/core/epoch/epoch_processing.go b/beacon-chain/core/epoch/epoch_processing.go index a1ea5a289759..7856b83d45dc 100644 --- a/beacon-chain/core/epoch/epoch_processing.go +++ b/beacon-chain/core/epoch/epoch_processing.go @@ -96,7 +96,7 @@ func ProcessRegistryUpdates(ctx context.Context, st state.BeaconState) (state.Be } // Collect validators eligible for activation and not yet dequeued for activation. - if helpers.IsEligibleForActivationUsingTrie(st, val) { + if helpers.IsEligibleForActivationUsingROVal(st, val) { eligibleForActivation = append(eligibleForActivation, primitives.ValidatorIndex(idx)) } diff --git a/beacon-chain/core/helpers/validators.go b/beacon-chain/core/helpers/validators.go index 9726b5d2d042..d213c136f197 100644 --- a/beacon-chain/core/helpers/validators.go +++ b/beacon-chain/core/helpers/validators.go @@ -470,13 +470,9 @@ func IsEligibleForActivation(state state.ReadOnlyCheckpoint, validator *ethpb.Va return isEligibleForActivation(validator.ActivationEligibilityEpoch, validator.ActivationEpoch, finalizedEpoch) } -// IsEligibleForActivationUsingTrie checks if the validator is eligible for activation. -func IsEligibleForActivationUsingTrie(state state.ReadOnlyCheckpoint, validator state.ReadOnlyValidator) bool { - cpt := state.FinalizedCheckpoint() - if cpt == nil { - return false - } - return isEligibleForActivation(validator.ActivationEligibilityEpoch(), validator.ActivationEpoch(), cpt.Epoch) +// IsEligibleForActivationUsingROVal checks if the validator is eligible for activation using the provided read only validator. +func IsEligibleForActivationUsingROVal(state state.ReadOnlyCheckpoint, validator state.ReadOnlyValidator) bool { + return isEligibleForActivation(validator.ActivationEligibilityEpoch(), validator.ActivationEpoch(), state.FinalizedCheckpointEpoch()) } // isEligibleForActivation carries out the logic for IsEligibleForActivation* diff --git a/beacon-chain/forkchoice/doubly-linked-tree/forkchoice.go b/beacon-chain/forkchoice/doubly-linked-tree/forkchoice.go index 941d3c47d51a..caacb7807528 100644 --- a/beacon-chain/forkchoice/doubly-linked-tree/forkchoice.go +++ b/beacon-chain/forkchoice/doubly-linked-tree/forkchoice.go @@ -267,11 +267,12 @@ func (f *ForkChoice) updateBalances() error { newBalances := f.justifiedBalances zHash := params.BeaconConfig().ZeroHash - for index, vote := range f.votes { + for index := 0; index < len(f.votes); index++ { // Skip if validator has been slashed if f.store.slashedIndices[primitives.ValidatorIndex(index)] { continue } + vote := &f.votes[index] // Skip if validator has never voted for current root and next root (i.e. if the // votes are zero hash aka genesis block), there's nothing to compute. if vote.currentRoot == zHash && vote.nextRoot == zHash { diff --git a/beacon-chain/p2p/gossip_topic_mappings.go b/beacon-chain/p2p/gossip_topic_mappings.go index 31733df0aa83..d88a4499ce2b 100644 --- a/beacon-chain/p2p/gossip_topic_mappings.go +++ b/beacon-chain/p2p/gossip_topic_mappings.go @@ -11,17 +11,17 @@ import ( // gossipTopicMappings represent the protocol ID to protobuf message type map for easy // lookup. -var gossipTopicMappings = map[string]proto.Message{ - BlockSubnetTopicFormat: ðpb.SignedBeaconBlock{}, - AttestationSubnetTopicFormat: ðpb.Attestation{}, - ExitSubnetTopicFormat: ðpb.SignedVoluntaryExit{}, - ProposerSlashingSubnetTopicFormat: ðpb.ProposerSlashing{}, - AttesterSlashingSubnetTopicFormat: ðpb.AttesterSlashing{}, - AggregateAndProofSubnetTopicFormat: ðpb.SignedAggregateAttestationAndProof{}, - SyncContributionAndProofSubnetTopicFormat: ðpb.SignedContributionAndProof{}, - SyncCommitteeSubnetTopicFormat: ðpb.SyncCommitteeMessage{}, - BlsToExecutionChangeSubnetTopicFormat: ðpb.SignedBLSToExecutionChange{}, - BlobSubnetTopicFormat: ðpb.BlobSidecar{}, +var gossipTopicMappings = map[string]func() proto.Message{ + BlockSubnetTopicFormat: func() proto.Message { return ðpb.SignedBeaconBlock{} }, + AttestationSubnetTopicFormat: func() proto.Message { return ðpb.Attestation{} }, + ExitSubnetTopicFormat: func() proto.Message { return ðpb.SignedVoluntaryExit{} }, + ProposerSlashingSubnetTopicFormat: func() proto.Message { return ðpb.ProposerSlashing{} }, + AttesterSlashingSubnetTopicFormat: func() proto.Message { return ðpb.AttesterSlashing{} }, + AggregateAndProofSubnetTopicFormat: func() proto.Message { return ðpb.SignedAggregateAttestationAndProof{} }, + SyncContributionAndProofSubnetTopicFormat: func() proto.Message { return ðpb.SignedContributionAndProof{} }, + SyncCommitteeSubnetTopicFormat: func() proto.Message { return ðpb.SyncCommitteeMessage{} }, + BlsToExecutionChangeSubnetTopicFormat: func() proto.Message { return ðpb.SignedBLSToExecutionChange{} }, + BlobSubnetTopicFormat: func() proto.Message { return ðpb.BlobSidecar{} }, } // GossipTopicMappings is a function to return the assigned data type @@ -44,27 +44,35 @@ func GossipTopicMappings(topic string, epoch primitives.Epoch) proto.Message { if epoch >= params.BeaconConfig().AltairForkEpoch { return ðpb.SignedBeaconBlockAltair{} } - return gossipTopicMappings[topic] + return gossipMessage(topic) case AttestationSubnetTopicFormat: if epoch >= params.BeaconConfig().ElectraForkEpoch { return ðpb.AttestationElectra{} } - return gossipTopicMappings[topic] + return gossipMessage(topic) case AttesterSlashingSubnetTopicFormat: if epoch >= params.BeaconConfig().ElectraForkEpoch { return ðpb.AttesterSlashingElectra{} } - return gossipTopicMappings[topic] + return gossipMessage(topic) case AggregateAndProofSubnetTopicFormat: if epoch >= params.BeaconConfig().ElectraForkEpoch { return ðpb.SignedAggregateAttestationAndProofElectra{} } - return gossipTopicMappings[topic] + return gossipMessage(topic) default: - return gossipTopicMappings[topic] + return gossipMessage(topic) } } +func gossipMessage(topic string) proto.Message { + msgGen, ok := gossipTopicMappings[topic] + if !ok { + return nil + } + return msgGen() +} + // AllTopics returns all topics stored in our // gossip mapping. func AllTopics() []string { @@ -81,7 +89,7 @@ var GossipTypeMapping = make(map[reflect.Type]string, len(gossipTopicMappings)) func init() { for k, v := range gossipTopicMappings { - GossipTypeMapping[reflect.TypeOf(v)] = k + GossipTypeMapping[reflect.TypeOf(v())] = k } // Specially handle Altair objects. GossipTypeMapping[reflect.TypeOf(ðpb.SignedBeaconBlockAltair{})] = BlockSubnetTopicFormat diff --git a/beacon-chain/p2p/gossip_topic_mappings_test.go b/beacon-chain/p2p/gossip_topic_mappings_test.go index efe7f00e6e4f..2c134f425fa6 100644 --- a/beacon-chain/p2p/gossip_topic_mappings_test.go +++ b/beacon-chain/p2p/gossip_topic_mappings_test.go @@ -15,7 +15,7 @@ func TestMappingHasNoDuplicates(t *testing.T) { params.SetupTestConfigCleanup(t) m := make(map[reflect.Type]bool) for _, v := range gossipTopicMappings { - if _, ok := m[reflect.TypeOf(v)]; ok { + if _, ok := m[reflect.TypeOf(v())]; ok { t.Errorf("%T is duplicated in the topic mapping", v) } m[reflect.TypeOf(v)] = true diff --git a/beacon-chain/sync/decode_pubsub.go b/beacon-chain/sync/decode_pubsub.go index a0e6070149c2..1ec9d079448a 100644 --- a/beacon-chain/sync/decode_pubsub.go +++ b/beacon-chain/sync/decode_pubsub.go @@ -16,7 +16,6 @@ import ( "github.com/prysmaticlabs/prysm/v5/config/params" "github.com/prysmaticlabs/prysm/v5/encoding/bytesutil" ethpb "github.com/prysmaticlabs/prysm/v5/proto/prysm/v1alpha1" - "google.golang.org/protobuf/proto" ) var errNilPubsubMessage = errors.New("nil pubsub message") @@ -52,7 +51,7 @@ func (s *Service) decodePubsubMessage(msg *pubsub.Message) (ssz.Unmarshaler, err if base == nil { return nil, p2p.ErrMessageNotMapped } - m, ok := proto.Clone(base).(ssz.Unmarshaler) + m, ok := base.(ssz.Unmarshaler) if !ok { return nil, errors.Errorf("message of %T does not support marshaller interface", base) } diff --git a/cmd/beacon-chain/main.go b/cmd/beacon-chain/main.go index b74f9d523671..19819c8e4316 100644 --- a/cmd/beacon-chain/main.go +++ b/cmd/beacon-chain/main.go @@ -212,6 +212,10 @@ func before(ctx *cli.Context) error { return errors.Wrap(err, "failed to set max fd limits") } + if err := features.ValidateNetworkFlags(ctx); err != nil { + return errors.Wrap(err, "provided multiple network flags") + } + return cmd.ValidateNoArgs(ctx) } diff --git a/cmd/prysmctl/validator/cmd.go b/cmd/prysmctl/validator/cmd.go index 54cb99a5e70d..fa25ca70df4b 100644 --- a/cmd/prysmctl/validator/cmd.go +++ b/cmd/prysmctl/validator/cmd.go @@ -175,6 +175,9 @@ var Commands = []*cli.Command{ if err := cmd.LoadFlagsFromConfig(cliCtx, cliCtx.Command.Flags); err != nil { return err } + if err := features.ValidateNetworkFlags(cliCtx); err != nil { + return err + } if err := tos.VerifyTosAcceptedOrPrompt(cliCtx); err != nil { return err } diff --git a/cmd/validator/main.go b/cmd/validator/main.go index c4584f955b07..cb0287a60c7f 100644 --- a/cmd/validator/main.go +++ b/cmd/validator/main.go @@ -193,6 +193,10 @@ func main() { return errors.Wrap(err, "failed to setup debug") } + if err := features.ValidateNetworkFlags(ctx); err != nil { + return errors.Wrap(err, "provided multiple network flags") + } + return cmd.ValidateNoArgs(ctx) }, After: func(ctx *cli.Context) error { diff --git a/config/features/config.go b/config/features/config.go index 925d32a89e28..3926af4a2f0d 100644 --- a/config/features/config.go +++ b/config/features/config.go @@ -20,6 +20,8 @@ The process for implementing new features using this package is as follows: package features import ( + "fmt" + "strings" "sync" "time" @@ -337,3 +339,25 @@ func logDisabled(flag cli.DocGenerationFlag) { } log.WithField(name, flag.GetUsage()).Warn(disabledFeatureFlag) } + +// ValidateNetworkFlags validates provided flags and +// prevents beacon node or validator to start +// if more than one network flag is provided +func ValidateNetworkFlags(ctx *cli.Context) error { + networkFlagsCount := 0 + for _, flag := range NetworkFlags { + if ctx.IsSet(flag.Names()[0]) { + networkFlagsCount++ + if networkFlagsCount > 1 { + // using a forLoop so future addition + // doesn't require changes in this function + var flagNames []string + for _, flag := range NetworkFlags { + flagNames = append(flagNames, "--"+flag.Names()[0]) + } + return fmt.Errorf("cannot use more than one network flag at the same time. Possible network flags are: %s", strings.Join(flagNames, ", ")) + } + } + } + return nil +} diff --git a/config/features/config_test.go b/config/features/config_test.go index 313945c0cc21..35ee7dfe5fcd 100644 --- a/config/features/config_test.go +++ b/config/features/config_test.go @@ -46,3 +46,50 @@ func TestConfigureBeaconConfig(t *testing.T) { c := Get() assert.Equal(t, true, c.EnableSlasher) } + +func TestValidateNetworkFlags(t *testing.T) { + // Define the test cases + tests := []struct { + name string + args []string + wantErr bool + }{ + { + name: "No network flags", + args: []string{"command"}, + wantErr: false, + }, + { + name: "One network flag", + args: []string{"command", "--sepolia"}, + wantErr: false, + }, + { + name: "Two network flags", + args: []string{"command", "--sepolia", "--holesky"}, + wantErr: true, + }, + { + name: "All network flags", + args: []string{"command", "--sepolia", "--holesky", "--mainnet"}, + wantErr: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Create a new CLI app with the ValidateNetworkFlags function as the Before action + app := &cli.App{ + Before: ValidateNetworkFlags, + Action: func(c *cli.Context) error { + return nil + }, + // Set the network flags for the app + Flags: NetworkFlags, + } + err := app.Run(tt.args) + if (err != nil) != tt.wantErr { + t.Errorf("ValidateNetworkFlags() error = %v, wantErr %v", err, tt.wantErr) + } + }) + } +} diff --git a/testing/spectest/mainnet/electra/operations/voluntary_exit_test.go b/testing/spectest/mainnet/electra/operations/voluntary_exit_test.go index ed87d6df8b87..c0f18adecf2f 100644 --- a/testing/spectest/mainnet/electra/operations/voluntary_exit_test.go +++ b/testing/spectest/mainnet/electra/operations/voluntary_exit_test.go @@ -7,6 +7,5 @@ import ( ) func TestMainnet_Electra_Operations_VoluntaryExit(t *testing.T) { - t.Skip("TODO: Electra") operations.RunVoluntaryExitTest(t, "mainnet") } diff --git a/testing/spectest/minimal/electra/operations/voluntary_exit_test.go b/testing/spectest/minimal/electra/operations/voluntary_exit_test.go index ab3098ed6b0c..ef54818ada01 100644 --- a/testing/spectest/minimal/electra/operations/voluntary_exit_test.go +++ b/testing/spectest/minimal/electra/operations/voluntary_exit_test.go @@ -7,6 +7,5 @@ import ( ) func TestMinimal_Electra_Operations_VoluntaryExit(t *testing.T) { - t.Skip("TODO: Electra") operations.RunVoluntaryExitTest(t, "minimal") }